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

more powerful optional text #150

Open
iomedico-beyer opened this issue Aug 2, 2022 · 12 comments
Open

more powerful optional text #150

iomedico-beyer opened this issue Aug 2, 2022 · 12 comments

Comments

@iomedico-beyer
Copy link

iomedico-beyer commented Aug 2, 2022

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

Currently optional text is only good for a plural s and the like.
So I have {int} cucumber(s) in my belly matches I have 1 cucumber in my belly.

I want to implement even less variants of identical test steps by having a more powerful optional text.

✨ What's your proposed solution?

  • I also wish I should not see the population (anymore) to match I should not see the population.
    (no extra space at the end of the text)
  • And I wish I should be (back) on the start page to match I should be on the start page.
    (no double space in the text)
  • And I wish sehe ich (wieder) den/die Breadcrumb(s) {string} to match sehe ich den Breadcrumb "Start".
    (still mixable with alternative text)

⛏ Have you considered any alternatives or workarounds?

Ugly workarounds:

  • cripple the test specs by adding extra spaces, e.g. I am on the start page (space at the end)
  • cripple the expressions by putting spaces in brackets, e.g. I am on the start page( again)
  • restricting test spec writer's language to be less natural (without “again”, etc.)
  • additional test step calling the other variant
  • workaround to convert such (more powerful) expressions to classic cucumber expressions (code here)

📚 Any additional context?

To be honest, I am somewhat surprised this doesn't work out of the box. See also the original discussion in another project.


This text was originally generated from a template, then edited by hand. You can modify the template here.

@mpkorstanje
Copy link
Contributor

mpkorstanje commented Aug 3, 2022

Given the effective EBNF from #44:

cucumber-expression     = (alternation
                           | optional
                           | parameter
                           | text)*
text                    = (- text-to-escape) | ('\', text-to-escape)
text-to-escape          = '(' | '{' | '/' | '\' 
alternation             = single-alternation, ('/', single_alternation)+
single-alternation      = ((text-in-alternative+, optional*) 
                            | (optional+, text-in-alternative+))+
text-in-alternative     = (- alternative-to-escape) | ('\', alternative-to-escape)
alternative-to-escape   = ' ' | '(' | '{' | '/' | '\'
optional                = '(', text-in-optional+, ')'
text-in-optional        = (- optional-to-escape) | ('\', optional-to-escape)
optional-to-escape      = '(' | ')' | '{' | '/' | '\'
parameter               = '{', name*, '}'
name                    = (- name-to-escape) | ('\', name-to-escape)
name-to-escape          = '{' | '}' | '(' | '/' | '\'

This would add

left-right-space-optional = ' ', '(', text-in-optional+, ')', ' '
left-space-optional       = ' ', '(', text-in-optional+, ')'
right-space-optional      = '(', text-in-optional+, ')', ' '
optional                  = '(', text-in-optional+, ')'

And the production rules

left-right-space-optional -> "(?:(?: " + rewrite(node[0]) + ... + rewrite(node[n-1]) + " )?| )"
left-space-optional ->"(?:(?: " + rewrite(node[0]) + ... + rewrite(node[n-1]) + ")?|)"
right-space-optional -> "(?:(?:" + rewrite(node[0]) + ... + rewrite(node[n-1]) + " )?|)"

I don't immediately see any problems with this. Unfortunately the current parser structure does implement this EBNF, but doesn't match its structure so adding it would be a little bit trickier.

@mpkorstanje
Copy link
Contributor

Maybe not:

(a) (b) (c)

Would produce right-space-optional, left-space-optional, left-space-optional and should match

a
a b
a c
a b c
b
b c
c

But won't match b because it is not a left-space-optional.

@mb73
Copy link

mb73 commented Aug 3, 2022

But won't match b because it is not a left-space-optional.

I’m not sure if I understand this. Would then cucumber(s) in my belly produce a right-space-optional and thus won’t match cucumber in my belly anymore?

@mpkorstanje
Copy link
Contributor

Worse, it would match cucumberin my belly because the space is now optional.

I'm not good at EBNFs, but I think left-space-optional and right-space-optional would have to be anchored to the start and end of a line. So that means pulling it up to the cucumber-expression.

cucumber-expression     = right-space-optional?, (alternation
                           | optional
                           | parameter
                           | text)*,
                          left-space-optional?

@mpkorstanje
Copy link
Contributor

Mmh. But what about this trainwreck

hello\ (beautiful)\ world/good\ bye

This should match

hello world
hello beautiful world
good bye

But that will be tricky because the \ would be captured by both the alternative-to-escape and left-right-space-optional.

Or with the first \ removed.

hello (beautiful)\ world/good\ bye

This should match

hello world
hello beautiful world
hello good bye

But the (beautiful)\ would have to be some right-space-optional variant that can be used in alternation.

@mb73
Copy link

mb73 commented Aug 3, 2022

cucumber-expression     = right-space-optional?, (alternation
                           | optional
                           | parameter
                           | text)*,
                          left-space-optional?

Would then a (b) c even be a cucumber-expression? According to that code, text can’t be at both sides of a …-space-optional, can it?

@mpkorstanje
Copy link
Contributor

No, you're right. I'm being incredibly sloppy.

cucumber-expression     = right-space-optional?, (alternation
                           | left-right-space-optional
                           | optional                           
                           | parameter
                           | text)*,
                          left-space-optional?

And then to handle variants like a (b)\ c/d:

alternation             = left-space-right-space-escaped-optional?, single-alternation, ('/', single_alternation)+, left-space-escaped-right-space-optional
single-alternation      = ((text-in-alternative+, optional-in-alternation*) 
                            | (optional-in-alternation+, text-in-alternative+))+
optional-in-alternation = left-space-escaped-right-space-escaped-optional | optional

I think.

Doesn't look like a simple change. 😆

@iomedico-beyer
Copy link
Author

iomedico-beyer commented Aug 9, 2022

I guess we could reduce the complexity of this enhancement a lot, if we introduced only left-space-optional but not right-space-optional. What do you think?

I guess the only downside of this simplified approach is that it does not help with braces at the beginning of the cucumber expression. On the other hand, I cannot find a good example where this matters.

@iomedico-beyer
Copy link
Author

Any hope for this one? 🙏

@mpkorstanje
Copy link
Contributor

Bit hard to say, I'm trying to get a bit of a workflow setup so I'm not the sole decision maker.

Though given that this is a relatively large amount of work (compared to the available time) and given that between duplicating step definitions and/or using regex there are reasonable alternatives if we do think this is a good idea, we might only solicit a pull request.

@iomedico-beyer
Copy link
Author

Thanks for the reply. I see. I am already using this feature a lot, enabled by my workaround (in TypeScript):

function toCucumberExpOrRegExp(description: string | RegExp) {
    return typeof description === 'string' ? description.replace(' (', '( ') : description;
}

There might be exotic cases where this does not work, though.

@mattwynne
Copy link
Member

FWIW this makes intuitive sense to me to support it. It's often frustrated me too.

I'm not deep enough in the implementation to understand the implied complexity of making it happen (so it's easy for me to say!) though I'd be happy to pair with someone who is and see if I can add anything.

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

No branches or pull requests

4 participants