Use single quotes for snippets #1298

danascheider · 2018-05-26T02:50:26Z

This PR addresses #1291 by changing snippets to use single quotes rather than double quotes.

Details

Previously, snippets looked like this:

Given("a user named {string}") do
  ...
end

Now, this snippet would look like this:

Given('a user named {string}') do
  ...
end

Motivation and Context

This PR fixes #1291, in which parentheses and brackets are not escaped correctly using double quotes. Following @aslakhellesoy's suggestion, instead of adding an extra escape character, I simply changed the snippets to use single quotes instead of double

How Has This Been Tested?

All Cucumber features and RSpec examples have been updated to incorporate the single quotes. I also added a Cucumber scenario testing that the characters are escaped correctly.

Types of changes

Refactor (code change that does not change external functionality)
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

I've added tests for my code
My change requires a change to the documentation.
I have updated the documentation accordingly.

luke-hill · 2018-05-29T07:46:07Z

Seems legit: Only thing I'd mention is this line (https://github.com/cucumber/cucumber-ruby/pull/1298/files#diff-2de4df90923d3667d3424beee025f01cR898) probably needs rewriting to specify in a bit more detail what the unit test does. But it's not a red/green line. So seems cool from me 👍

xtrasimplicity · 2018-05-31T03:45:55Z

The documentation PR has been merged, so we should probably merge this one, too, for consistency. :)

aslakhellesoy

To my knowledge, there aren't any widely accepted conventions for the use of single or double qutes.

With Cucumber I prefer to use double quotes, because I use single quotes in steps relatively often:

Given Aslak has joined Dana's game

With double quotes I don't have to escape them in the Cucumber Expression.

I mentioned in #1291 that using single quotes would be a workaround. I didn't mean that it would be a general solution to the problem.

I think a better fix would be to keep the double quotes, and escape any \ characters in the expression with \\.

aslakhellesoy · 2018-05-31T05:14:41Z

lib/cucumber/glue/snippet.rb

@@ -88,7 +88,7 @@ def typed_pattern
        def to_s
          header = generated_expressions.each_with_index.map do |expr, i|
            prefix = i == 0 ? '' : '# '
-            "#{prefix}#{code_keyword}(\"#{expr.source}\") do#{parameters(expr)}"
+            "#{prefix}#{code_keyword}('#{expr.source}') do#{parameters(expr)}"


Have you considered the case where expr.source contains a single quote?

Or, if we keep the double-quoted strings, the case where expr.source contains a \ character.

Ooh! Good point. I actually agree with everything you’re saying and would also prefer the double quotes to stay. I just misread your other comments and took them to mean you preferred this way, and deferred to that even though I didn’t agree 😂

I’m making myself a note to submit new documentation changes tomorrow morning and change this PR right away.

aslakhellesoy · 2018-05-31T05:14:50Z

features/docs/defining_steps/snippets.feature

+        pending # Write code here that turns the phrase above into concrete actions
+      end
+
+      Given('another step with \{brackets}') do


I think it's called curly braces, or just braces

danascheider · 2018-05-31T20:43:05Z

@aslakhellesoy It looks to me like implementing the escape will involve changes to Cucumber Expressions since expr.source might contain params in curly braces. I might be able to find a workaround for this using expr.parameter_names but it might be easier to add the escape character in Cucumber expressions before the params are interpolated. WDYT?

stale · 2018-07-30T20:44:22Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs.

aslakhellesoy · 2018-07-30T21:14:20Z

Bump :-)

stale · 2018-09-28T21:41:38Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs.

danascheider · 2018-09-30T20:08:15Z

Bump. This is still on my radar, I'm just moving to Australia next Sunday so things have been hectic.

aslakhellesoy · 2018-09-30T21:06:09Z

Take your time @danascheider - good luck with the move. We'll keep this one waiting for you down under.

danascheider · 2018-10-24T22:44:46Z

@aslakhellesoy I'm closing this PR since we decided on a different implementation. I'll open a new one when I have that implementation fleshed out more fully.

lock · 2019-10-24T23:20:35Z

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.

danascheider added 3 commits May 25, 2018 19:14

Test for single quotes in snippets and include edge cases

69ad1a8

Conflate snippet features and fix quote thing

0499d3f

Fix failing tests

e6209b3

ghost assigned danascheider May 26, 2018

ghost added the status: work in progress (WIP) label May 26, 2018

danascheider mentioned this pull request May 26, 2018

Update docs to reflect single quoted strings now used in Ruby snippets cucumber/docs#174

Merged

xtrasimplicity approved these changes May 31, 2018

View reviewed changes

aslakhellesoy suggested changes May 31, 2018

View reviewed changes

stale bot added the ⌛ stale Will soon be closed by stalebot unless there is activity label Jul 30, 2018

stale bot removed the ⌛ stale Will soon be closed by stalebot unless there is activity label Jul 30, 2018

stale bot added the ⌛ stale Will soon be closed by stalebot unless there is activity label Sep 28, 2018

aslakhellesoy removed the ⌛ stale Will soon be closed by stalebot unless there is activity label Sep 28, 2018

danascheider closed this Oct 24, 2018

ghost removed the status: work in progress (WIP) label Oct 24, 2018

danascheider deleted the 1291-use-single-quotes-in-snippets branch October 24, 2018 22:44

lock bot locked as resolved and limited conversation to collaborators Oct 24, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use single quotes for snippets #1298

Use single quotes for snippets #1298

danascheider commented May 26, 2018 •

edited

Loading

luke-hill commented May 29, 2018 •

edited

Loading

xtrasimplicity commented May 31, 2018 •

edited

Loading

aslakhellesoy left a comment

aslakhellesoy May 31, 2018

aslakhellesoy May 31, 2018

danascheider May 31, 2018

danascheider May 31, 2018

aslakhellesoy May 31, 2018

danascheider commented May 31, 2018

stale bot commented Jul 30, 2018

aslakhellesoy commented Jul 30, 2018

stale bot commented Sep 28, 2018

danascheider commented Sep 30, 2018

aslakhellesoy commented Sep 30, 2018

danascheider commented Oct 24, 2018

lock bot commented Oct 24, 2019

Use single quotes for snippets #1298

Use single quotes for snippets #1298

Conversation

danascheider commented May 26, 2018 • edited Loading

Details

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

luke-hill commented May 29, 2018 • edited Loading

xtrasimplicity commented May 31, 2018 • edited Loading

aslakhellesoy left a comment

Choose a reason for hiding this comment

aslakhellesoy May 31, 2018

Choose a reason for hiding this comment

aslakhellesoy May 31, 2018

Choose a reason for hiding this comment

danascheider May 31, 2018

Choose a reason for hiding this comment

danascheider May 31, 2018

Choose a reason for hiding this comment

aslakhellesoy May 31, 2018

Choose a reason for hiding this comment

danascheider commented May 31, 2018

stale bot commented Jul 30, 2018

aslakhellesoy commented Jul 30, 2018

stale bot commented Sep 28, 2018

danascheider commented Sep 30, 2018

aslakhellesoy commented Sep 30, 2018

danascheider commented Oct 24, 2018

lock bot commented Oct 24, 2019

danascheider commented May 26, 2018 •

edited

Loading

luke-hill commented May 29, 2018 •

edited

Loading

xtrasimplicity commented May 31, 2018 •

edited

Loading