Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Delegate snippet-type CLI options to the snippet classes. #409

Merged
merged 3 commits into from

4 participants

@rdvdijk

The CLI option description and examples are now delegated to the snippet classes.

    -I, --snippet-type TYPE          Use different snippet type (Default: regexp). Available types:
                                     legacy : Snippets without parentheses e.g. Given /^missing step$/
                                     percent: Snippets with percent regexp e.g. Given %r{^missing step$}
                                     regexp : Snippets with parentheses    e.g. Given(/^missing step$/)

I've ordered the snippet-types alphabetically to make it more predictable (I think #each_pair orders differently in 1.8 compared to 1.9 and 2.0).

@rdvdijk

Failing for 1.8... :unamused:

undefined method `<=>' for :legacy:Symbol
@os97673 os97673 merged commit 588545f into from
@abotalov

I haven't tried this PR but could you tell why syntax Given /^missing step$/ is called legacy? Is it considered bad practice?

And what was the reason to add other syntaxes?

@os97673
Collaborator

@abotalov legacy since it is the old one. We do not use it now (by default) because ruby issues warning for such method call (it wants parenthesis around params).

@abotalov

I use Ruby 2.0.0p0 and Cucumber 1.2.3 and don't see any warnings

@mattwynne
Owner
@abotalov

Syntax of :legacy is shorter than of :regexp or :percent.

Is :legacy still considered a good/best practice by Cucumber team?

  • If yes, probably it should be default instead of :regexp and :classic is a better name.
  • If no, could you tell, what is the reason it's not considered a good practice? I read #328 but it seems to me it doesn't affect majority of Cucumber users that run Cucumber as cucumber features
@mattwynne
Owner

The :legacy option is not considered good practice by me because it creates code that generates warnings from Ruby. Those warnings are useful to people who want to keep their Ruby code maintainable, and we do not want to add noise to their warnings by default.

What's the harm in it @abotalov? What are you worried about?

I still think we should rename :legacy to :classic

@abotalov

I haven't tried this PR when I wrote that so I thought it will break code of all people that use Cucumber as I thought this default means all stepdefs should be in this style (:regexp by default). I now tried Cucumber master and it seems Cucumber doesn't require all step defs to be in the same style and this change seems to affect only style in which new steps are generated by Cucumber.

So currently I'm not worried :smile:. Thank you for response!

@mattwynne mattwynne referenced this pull request from a commit
@mattwynne mattwynne Rename legacy snippet to classic
As I've been banging on about in #409
757bf9f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
5 lib/cucumber/cli/options.rb
@@ -1,5 +1,6 @@
require 'cucumber/cli/profile_loader'
require 'cucumber/formatter/ansicolor'
+require 'cucumber/rb_support/rb_language'
module Cucumber
module Cli
@@ -241,9 +242,7 @@ def parse!(args)
end
opts.on("-I", "--snippet-type TYPE",
"Use different snippet type (Default: regexp). Available types:",
- "regexp : Snippets with parentheses, e.g. \"When(/^missing step$/) do\"",
- "legacy : Snippets without parentheses, e.g. \"When /^missing step$/ do\"",
- "percent : Snippets with percent regexp, e.g. \"When %r{^missing step$} do\"") do |v|
+ *Cucumber::RbSupport::RbLanguage.cli_snippet_type_options) do |v|
@options[:snippet_type] = v.to_sym
end
View
19 lib/cucumber/rb_support/rb_language.rb
@@ -170,13 +170,20 @@ def check_nil(o, proc)
end
end
+ SNIPPET_TYPES = {
+ :regexp => Snippet::Regexp,
+ :legacy => Snippet::Legacy,
+ :percent => Snippet::Percent
+ }
+
def typed_snippet_class(type)
- type ||= :regexp
- {
- :regexp => Snippet::Regexp,
- :legacy => Snippet::Legacy,
- :percent => Snippet::Percent
- }.fetch(type)
+ SNIPPET_TYPES.fetch(type || :regexp)
+ end
+
+ def self.cli_snippet_type_options
+ SNIPPET_TYPES.keys.sort_by(&:to_s).map do |type|
+ SNIPPET_TYPES[type].cli_option_string(type)
+ end
end
end
end
View
26 lib/cucumber/rb_support/snippet.rb
@@ -14,7 +14,15 @@ def initialize(code_keyword, pattern, multiline_argument_class)
end
def to_s
- "#{code_keyword}#{typed_pattern} #{do_block}"
+ "#{step} #{do_block}"
+ end
+
+ def step
+ "#{code_keyword}#{typed_pattern}"
+ end
+
+ def self.cli_option_string(type)
+ "%-7s: %-28s e.g. %s" % [type, description, example]
end
private
@@ -59,24 +67,40 @@ def multiline_argument_class?
multiline_argument_class == Ast::Table
end
+ def self.example
+ new("Given", "missing step", nil).step
+ end
+
end
class Regexp < BaseSnippet
def typed_pattern
"(/^#{pattern}$/)"
end
+
+ def self.description
+ "Snippets with parentheses"
+ end
end
class Legacy < BaseSnippet
def typed_pattern
" /^#{pattern}$/"
end
+
+ def self.description
+ "Snippets without parentheses"
+ end
end
class Percent < BaseSnippet
def typed_pattern
" %r{^#{pattern}$}"
end
+
+ def self.description
+ "Snippets with percent regexp"
+ end
end
end
Something went wrong with that request. Please try again.