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

Return optionals as part of a Cucumber Expression match #125

Open
aslakhellesoy opened this issue May 26, 2022 · 4 comments
Open

Return optionals as part of a Cucumber Expression match #125

aslakhellesoy opened this issue May 26, 2022 · 4 comments

Comments

@aslakhellesoy
Copy link
Contributor

aslakhellesoy commented May 26, 2022

🤔 What's the problem you're trying to solve?

I want to provide syntax highlighting (LSP semantic tokens) for the parts of a Gherkin step that is optional in the corresponding Cucumber Expression.

✨ What's your proposed solution?

Use named capture groups for optionals, as explained in cucumber/language-service#57

The return type of match would change from Argument[] | null to Match | null:

export type Match = {
  arguments: readonly Argument[]
  optionals: readonly Optional[]
}

export type Optional = {
  group: Group
}

⛏ Have you considered any alternatives or workarounds?

If we want to support this, I can't think of any other option.

@mpkorstanje
Copy link
Contributor

Currently you can not assign special meaning to groups because every possible regex generated by a Cucumber expression is also a regular valid regular regex that may be provided by a user.

For example the regex:

A(?<Special-String> non-optional) pattern

Would be capturing when used with Cucumber, but non capturing in the Language Server. And while it is unlikely a user may have guessed Special-String, it is an indication that this abstraction is leaking.

Now I'm not opposed to adding named group support to the tree regex. These names could be used when generating step definitions based of regular regexes for example.

However the proposed Special-String should not be used in production code. So consider generating a different regex patterns when transforming the AST to a Cucumber expression while using the langue server.

This may require some refactoring.

@aslakhellesoy
Copy link
Contributor Author

aslakhellesoy commented May 26, 2022

I wrote in cucumber/language-service#57 that we could change TreeRegexp. After thinking about it a bit more, I think the correct place to do this is in rewriteOptional:

We could pass a makeOptionalGroup: (regex: string, count: number) => string constructor argument to CucumberExpression (defaulting to this:

(regex, count) => `(?:${regex})?`

That way, the language server could pass its own:

(regex, count) => `(?<optional-${count}>${regex})`

@mpkorstanje
Copy link
Contributor

That interface is very language server specific.

For example:

  • count is not a concept that occurs in cucumber expressions. It exists only for the language server.
  • This only and specifically replaces optionals.

Ideally we define the Cucumber expression API in its own domain terms.

So I think some refactoring is in order.

@mpkorstanje
Copy link
Contributor

It does come to mind that currently we also assume that all capture groups are parameters. If you write optionals as capturing then match won't work anymore.

Maybe you can extract the code that rewrites the AST, add support for named groups to tree-regex and then use those in a class that is not CucumberExpression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants