diff options
author | Marc A. Paradise <marc.paradise@gmail.com> | 2019-06-06 15:24:49 -0400 |
---|---|---|
committer | Marc A. Paradise <marc.paradise@gmail.com> | 2019-06-06 15:31:10 -0400 |
commit | 840b5a627d708cef7065602a581547ac07acd64e (patch) | |
tree | 1bb050fc0c7fa15f61c42ee932e92b799c39f0d7 | |
parent | 145ba5f6cdccbb0775d5a65885b80705bc9fa87a (diff) | |
download | mixlib-cli-840b5a627d708cef7065602a581547ac07acd64e.tar.gz |
Code review updates for deprecated_optionMIXLIB-CLI-63/deprecated-option-support
Signed-off-by: Marc A. Paradise <marc.paradise@gmail.com>
-rw-r--r-- | lib/mixlib/cli.rb | 20 | ||||
-rw-r--r-- | lib/mixlib/cli/formatter.rb | 3 | ||||
-rw-r--r-- | spec/mixlib/cli/formatter_spec.rb | 43 |
3 files changed, 55 insertions, 11 deletions
diff --git a/lib/mixlib/cli.rb b/lib/mixlib/cli.rb index 5075dec..1a83a11 100644 --- a/lib/mixlib/cli.rb +++ b/lib/mixlib/cli.rb @@ -220,13 +220,6 @@ module Mixlib # the values supplied by the user, see #config. attr_accessor :options - # Warn if a duplicate item has been added to the options list. - # This is intended to notify developers when they're using - # mixlib-cli incorrectly - either by specifying an option more than once, - # or more commonly from re-specifying options that have been inhereted from a - # shared base class. - attr_accessor :warn_on_duplicates - # A Hash containing the values supplied by command line options. # # The behavior and contents of this Hash vary depending on whether @@ -396,7 +389,8 @@ module Mixlib # # @return NilClass def handle_deprecated_options(show_deprecations) - config.keys.each do |opt_key| + merge_in_values = {} + config.each_key do |opt_key| opt_cfg = options[opt_key] # Deprecated entries do not have defaults so no matter what # separate_default_options are set, if we see a 'config' @@ -409,7 +403,10 @@ module Mixlib # the declared value mapper (defaults to return the same value if caller hasn't # provided a mapper). deprecated_val = config[opt_key] - config[replacement_key] = opt_cfg[:value_mapper].call(deprecated_val) + + # We can't modify 'config' since we're iterating it, apply updates + # at the end. + merge_in_values[replacement_key] = opt_cfg[:value_mapper].call(deprecated_val) config.delete(opt_key) unless opt_cfg[:keep] end @@ -420,14 +417,15 @@ module Mixlib puts "#{display_name}: #{opt_cfg[:description]}" end end + config.merge!(merge_in_values) nil end def build_option_arguments(opt_setting) arguments = Array.new - arguments << opt_setting[:short] unless opt_setting[:short].nil? - arguments << opt_setting[:long] unless opt_setting[:long].nil? + arguments << opt_setting[:short] if opt_setting[:short] + arguments << opt_setting[:long] if opt_setting[:long] if opt_setting.key?(:description) description = opt_setting[:description].dup description << " (required)" if opt_setting[:required] diff --git a/lib/mixlib/cli/formatter.rb b/lib/mixlib/cli/formatter.rb index b89090d..fef8503 100644 --- a/lib/mixlib/cli/formatter.rb +++ b/lib/mixlib/cli/formatter.rb @@ -10,6 +10,9 @@ module Mixlib # @return [String] the formatted flag name as described above def self.combined_option_display_name(short, long) usage = "" + # short/long may have an argument (--long ARG) + # splitting on " " and taking first ensures that we get just + # the flag name without the argument if one is present. usage << short.split(" ").first if short usage << "/" if long && short usage << long.split(" ").first if long diff --git a/spec/mixlib/cli/formatter_spec.rb b/spec/mixlib/cli/formatter_spec.rb new file mode 100644 index 0000000..5500528 --- /dev/null +++ b/spec/mixlib/cli/formatter_spec.rb @@ -0,0 +1,43 @@ + +require "mixlib/cli/formatter" + +describe Mixlib::CLI::Formatter do + Formatter = Mixlib::CLI::Formatter + context "combined_option_display_name" do + it "converts --option with short -o to '-s/--option'" do + expect(Formatter.combined_option_display_name("-o", "--option")).to eql "-o/--option" + end + + it "converts --option with no short to '--option'" do + expect(Formatter.combined_option_display_name(nil, "--option")).to eql "--option" + end + it "converts short -o with no long option to '-o'" do + expect(Formatter.combined_option_display_name("-o", nil)).to eql"-o" + end + + it "converts options the same way even with an argument present" do + expect(Formatter.combined_option_display_name("-o arg1", "--option arg1")).to eql "-o/--option" + end + + it "converts options to a blank string if neither short nor long are present" do + expect(Formatter.combined_option_display_name(nil, nil)).to eql "" + end + end + + context "friendly_opt_list" do + it "for a single item it quotes it and returns it as a string" do + expect(Formatter.friendly_opt_list(%w{hello})).to eql "'hello'" + end + it "for two items returns ..." do + expect(Formatter.friendly_opt_list(%w{hello world})).to eql "'hello' or 'world'" + end + it "for three items returns..." do + expect(Formatter.friendly_opt_list(%w{hello green world})).to eql "'hello', 'green', or 'world'" + end + it "for more than three items creates a list in the same was as three items" do + expect(Formatter.friendly_opt_list(%w{hello green world good morning})).to eql "'hello', 'green', 'world', 'good', or 'morning'" + end + + end + +end |