Suggested regexes are bad and result in ambiguous matches

[rwz]

Imagine I'm writing a new scenario that includes something similar to this:

When I press "Submit" button
Then I should see "can't be blank" validation error on "Email"

first time I run this new scenario, cucumber will suggest me code snippets to start implementing missing steps. The snippet for validation step would be

Then(/^I should see "(.*?)" validation error on "(.*?)"$/) do |arg1, arg2|
  pending # express the regexp above with the code you wish you had
end

Later I add another scenario that includes this:

Then I should see "foobar"

and cucumber, again, helpfully suggest a snippet to start implementing the step:

Then(/^I should see "(.*?)"$/) do |arg1|

The problem is, the suggested regexp also matches everything, a previously defined validation step does resulting with lot of previously green steps suddenly go red with Cucumber::Ambiguous error.

[rwz]

Simplest solution would be to suggest a more robust regexp that does't allow quotes inside quoted value:

Then /^I should see "([^"]*)"$/ do |arg1|

or even better:

Then /^I should see "((?:\\"|[^"])*)"$/ do |arg1|

The second option allows escaped quotes inside a quoted value, like this:

Then I should see "press \"proceed\" to continue"

I'd personally prefer the latter, but I could see how such regexp might confuse a lot of people not used to seeing things like this.

[mattwynne]

See #237 for background on this.

TBH I'd prefer not to change this back at the moment, as we have bigger fish to fry with the 2.0 release. When we're done with that I'd like to concentrate on bringing in Turnip-style placeholders which hide the regexp altogether.

[aslakhellesoy]

I think turnip-style patterns would be a step in the right direction for all cucumber implementations. Behat 3 already supports them.

Does it make sense to create a small turnip-pattern library for the various programming languages? /cc @jnicklas @everzet @gasparnagy @jbpros

[mattwynne]

I'm going to close this PR. I do appreciate it, thanks, but I'd prefer to keep the generated snippets non-intimidating for the 80/20 use-case. I hope we can use your more robust ones when #708 is implemented.

Any objections?

[aslakhellesoy]

@mattwynne are you saying ([^"]*) is more intimidating than (.*?)? (I would disagree).

Cucumber suggested ([^"]*) in snippets for many years until we accepted a poor patch that switched it to (.*?) - a regression IMO.

I think we should switch it back - it should be easy to do.

[aslakhellesoy]

Let me elaborate on why I disagree that ([^"]*) is more intimidating than (.*?) and why I think this PR should be applied.

Let's assume we have two kinds of users - those who know enough regular expressions to understand both styles, and those who don't. There might also be some people who understand one style, but not the other, but that is probably a very small percentage, so let's ignore them for the sake of this argument.

The users who do understand regular expressions are unlikely to find either style more intimidating than the other.

The users who don't understand regular expressions are likely to find both styles equally intimidating.

Of course, I'm only hypothesising here, but so are you @mattwynne. We don't have data to support any of these hypotheses.

Since we don't have data to support either hypothesis we should decide what style to use based on which one is most correct and causes the least surprise during use. I would argue that even if we did have data to support the claim that people find ([^"]*) more intimidating than (.*?), we should still use ([^"]*) because it is more correct.

@rwz has already made a good case that the (.*?) can cause surprising behaviour while the original ([^"]*) will not.

[rwz]

I'd say that .*? is arguably more confusing than straightforward [^"]*. In my experience a lot of people do not understand the concept of non-greedy ? modifier for *.

I find myself explaining this regexp to others a lot more often than [^"]*.

[mattwynne]

Oh, the only intimidation I'm thinking about is in the aesthetic. I've observed that people who don't understand regexps at all find the [^"]* form ugly and therefore intimidating. However, I'm convinced by both your points of view, and we'll merge this in for release with 2.0.

[mattwynne]

Thanks @rwz!

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.