Skip to content
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

Incorrect extracting of {string} #7

Closed
DDtKey opened this issue Feb 3, 2022 · 4 comments · Fixed by #8 or cucumber-rs/cucumber#204
Closed

Incorrect extracting of {string} #7

DDtKey opened this issue Feb 3, 2022 · 4 comments · Fixed by #8 or cucumber-rs/cucumber#204
Assignees
Labels
bug Something isn't working
Milestone

Comments

@DDtKey
Copy link

DDtKey commented Feb 3, 2022

In accordance with cucumber-expressions - {string} should return only text between quotes.

Parameter Type Description
{string} Matches single-quoted or double-quoted strings, for example "banana split" or 'banana split' (but not banana split). Only the text between the quotes will be extracted. The quotes themselves are discarded. Empty pairs of quotes are valid and will be matched and passed to step code as empty strings.

But in current version there are quotes in String value

@tyranron tyranron added the bug Something isn't working label Feb 3, 2022
@ilslv
Copy link
Member

ilslv commented Feb 4, 2022

@tyranron the trouble with this issue is that inside cucumber crate we map regex capturing groups one to one into callback parameters.

#[given(expr = "{param1} {param2} {param3}")]
fn callback(param1: i32, param2: i32, param3: i32) { ... }

So every parameter has to be represented with single capturing group and in case group didn't match we return empty string to avoid messing up the order.

We have to provide regex, that will match both "text" and 'text' into single group containing text and I don't think this is possible with regex crate, as it would require remembering starting quote and matching until we find the same one.

Possible solutions to this issue

  1. Return 2 groups with special names. Only 1 of them will match and handle another one inside cucumber crate based on a special name.
  2. Require users to always escape " inside '...' and vice versa.
  3. Leave it as is.

I like option 2 the most, because it simplifies the matching group, while not contradicting existing vague description of the language. But to be fair actual implementation returns group containing 3 capturing groups: ("([^"\\]*(\\.[^"\\]*)*)"|'([^'\\]*(\\.[^'\\]*)*)') and " ' " is completely valid parameter.

In case of Java, they are handling strings as some kind of special way from what I can tell. This is the only parameter consisting of 2 separate regex groups.

@tyranron
Copy link
Member

tyranron commented Feb 4, 2022

@ilslv thank you for the clear explanation.

We cannot deviate much from what official implementation does, so the option 2 is not an option, unfortunately.

So, as the Java implementation does, we should handle {string} in a special way too on the cucumber side. Either with matching 2 capturing groups (as you've described), or by matching the whole, including quotes (as we do now), following by some post-procession (trimming quotes) in machinery before feeding it into the function.

@ilslv ilslv linked a pull request Feb 8, 2022 that will close this issue
15 tasks
tyranron added a commit that referenced this issue Feb 10, 2022
…, #7)

Co-authored-by: Kai Ren <tyranron@gmail.com>
@tyranron tyranron added this to the 0.2.0 milestone Feb 10, 2022
tyranron pushed a commit to cucumber-rs/cucumber that referenced this issue Feb 10, 2022
- fix `{string}` parameter returning enclosing quotes (cucumber-rs/cucumber-expressions#7)
@tyranron
Copy link
Member

@DDtKey this is fixed now in 0.12.0 cucumber release.

Additionally, we have extended this case to custom parameters as well. So now, similar patterns where multiple capturing groups are required may be used for them too.

@DDtKey
Copy link
Author

DDtKey commented Feb 10, 2022

Thanks for the quick update! 💥

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment