Skip to content

Support taking regex captures as arguments to the step definition's function #159

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Aug 18, 2017
Merged

Conversation

muggenhor
Copy link
Contributor

@muggenhor muggenhor commented Aug 12, 2017

This feature depends on the availability of C++11 (or more recent). It's main purpose is to support the transformation from code such as this:

GIVEN("some (\\d+) and a \"([^\"]+)\"") {
    REGEX_PARAM(unsigned, number);
    REGEX_PARAM(std::string, string);
}

to use the more familiar syntax present in this code:

GIVEN("some (\\d+) and a \"([^\"]+)\"", (const unsigned number, const std::string& string)) {
}

Currently all captures from the regex that aren't passed as function parameters can still be extracted by means of the REGEX_PARAM macro.

If C++11 is unavailable then this feature is just unavailable and all existing code should continue to work as is.

In the future it may be desirable to check, at compile-time, that the amount of captures in the regex equals the amount of function parameters. I have some prototype code I've been playing with that suggests this is relatively doable. It currently depends on "relaxed constexpr" support (C++14), constexpr lambdas (C++ 17) and "constexpr if". I may be able to rewrite this prototype to rely on just the C++14 features, but that's for the future. Heck, it may even be possible to use recursion and rely only on C++11, but that's bound to produce very difficult to read code.

This is intended to provide a replacement for the use of the REGEX_PARAM
macro and to pass the parameters more naturally: as function parameters.
On C++11 (and beyond), support extracting of the regex' captures to the
arguments of the step function. This enables a more natural syntax to
receive these than using the REGEX_PARAM macro.

Currently all captures from the regex that aren't passed as parameters
can still be extracted by means of the REGEX_PARAM macro.

In the future it may be desirable to check, at compile-time, that the
amount of captures in the regex equals the amount of function
parameters. Unfortunately this is relatively doable in C++17, because of
the presence of constexpr lambdas, more difficult in C++14, and probably
impossible in C++11.
This prevents nasty surprises where we might try to convert to a const
type, which would fail in our current 'fromString' implementation.
@muggenhor muggenhor added this to the Backlog milestone Aug 12, 2017
Because MSVC's value for __cplusplus doesn't even claim C++98 support.
That's actually good, because it doesn't even support that yet. It does
however preclude us from using that to perform feature detection: so
don't.
@muggenhor muggenhor modified the milestones: v0.5, Backlog Aug 12, 2017
@konserw
Copy link
Contributor

konserw commented Aug 13, 2017

Looks very good to me 👍 I'm also happy to see modern C++ features in good use.

This gets rid of both the runtime overhead associated with dynamic_cast
and the need to explicitly specify the Derived class to cast to. Instead
we pass an "improved" 'this' around: 'that'.
This gets rid of the FunctionSignature and FunctionArgs helper classes
by using a pointer-to-member-function and being able to get the
arguments as a pack directly. As a result the function signature no
longer has to be specified explicitly as a template parameter, it's
instead determined by the compiler using template type deduction.

This further simplifies the C++98 code to be ridiculously
straightforward and the C++11 is simpler because it no longer has to be
C++98 compatible.
Copy link
Member

@paoloambrosio paoloambrosio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was an experiment from a few months ago, but I never ported it to the Cucumber-CPP code base: https://github.com/paoloambrosio/experiments/tree/cuke-cpp-reg-macro

The syntax is slightly different but the result is similar. Don't think there is anything in there that is not present in this PR.

Good job!

@muggenhor muggenhor self-assigned this Aug 18, 2017
@muggenhor muggenhor merged commit 0374a5c into cucumber:master Aug 18, 2017
muggenhor added a commit that referenced this pull request Aug 18, 2017
@muggenhor muggenhor deleted the func-args branch August 18, 2017 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants