From 552319d566b65f63fd2f11267ad4d177930a69e0 Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Mon, 13 May 2019 19:16:50 -0700 Subject: Print out human readable lists of allowed CLI options Give a nice friendly sentence and not an array. This uses the code we nuked from chef/chef that did the same. Signed-off-by: Tim Smith --- lib/mixlib/cli.rb | 16 +++++++++++++--- spec/mixlib/cli_spec.rb | 8 +++++++- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/lib/mixlib/cli.rb b/lib/mixlib/cli.rb index cfc642d..4141e4a 100644 --- a/lib/mixlib/cli.rb +++ b/lib/mixlib/cli.rb @@ -1,6 +1,6 @@ # # Author:: Adam Jacob () -# Copyright:: Copyright (c) 2008-2016 Chef Software, Inc. +# Copyright:: Copyright (c) 2008-2019 Chef Software, Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -243,7 +243,7 @@ module Mixlib end if config[opt_key] && !opt_value[:in].include?(config[opt_key]) reqarg = opt_value[:short] || opt_value[:long] - puts "#{reqarg}: #{config[opt_key]} is not included in the list ['#{opt_value[:in].join("', '")}'] " + puts "#{reqarg}: #{config[opt_key]} is not one of the allowed values: #{friendly_opt_list(opt_value[:in])}" puts @opt_parser exit 2 end @@ -313,7 +313,7 @@ module Mixlib if opt_setting.key?(:description) description = opt_setting[:description].dup description << " (required)" if opt_setting[:required] - description << " (valid options are: ['#{opt_setting[:in].join("', '")}'])" if opt_setting[:in] + description << " (valid options: #{friendly_opt_list(opt_setting[:in])})" if opt_setting[:in] opt_setting[:description] = description arguments << description end @@ -321,6 +321,16 @@ module Mixlib arguments end + # @private + # @param opt_arry [Array] + # + # @return [String] a friendly quoted list of items complete with "and" + def friendly_opt_list(opt_array) + opts = opt_array.map { |x| "'#{x}'" } + return opts.join(" and ") if opts.size < 3 + opts[0..-2].join(", ") + " and " + opts[-1] + end + def self.included(receiver) receiver.extend(Mixlib::CLI::ClassMethods) receiver.extend(Mixlib::CLI::InheritMethods) diff --git a/spec/mixlib/cli_spec.rb b/spec/mixlib/cli_spec.rb index c9bc5e4..3adda52 100644 --- a/spec/mixlib/cli_spec.rb +++ b/spec/mixlib/cli_spec.rb @@ -228,7 +228,13 @@ describe Mixlib::CLI do TestCLI.option(:inclusion, short: "-i val", in: %w{one two}, description: "desc", required: false) @cli = TestCLI.new @cli.parse_options(["-i", "one"]) - expect(@cli.options[:inclusion][:description]).to eql("desc (valid options are: ['one', 'two'])") + expect(@cli.options[:inclusion][:description]).to eql("desc (valid options: 'one' and 'two')") + end + + it "Raises SystemExit when the provided value is not a member of the :in array" do + TestCLI.option(:inclusion, short: "-i val", in: %w{one two}, description: "desc", required: false) + @cli = TestCLI.new + expect(lambda { @cli.parse_options(["-i", "three"]) }).to raise_error(SystemExit) end it "doesn't exit if a required option is specified" do -- cgit v1.2.1 From 32ee9105705e3484ddcb2953ed8675c50d5035e7 Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Mon, 13 May 2019 19:23:59 -0700 Subject: Add more specs and use or not and This seems to make more sense Signed-off-by: Tim Smith --- lib/mixlib/cli.rb | 6 +++--- spec/mixlib/cli_spec.rb | 18 +++++++++++++----- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/lib/mixlib/cli.rb b/lib/mixlib/cli.rb index 4141e4a..f3598e6 100644 --- a/lib/mixlib/cli.rb +++ b/lib/mixlib/cli.rb @@ -324,11 +324,11 @@ module Mixlib # @private # @param opt_arry [Array] # - # @return [String] a friendly quoted list of items complete with "and" + # @return [String] a friendly quoted list of items complete with "or" def friendly_opt_list(opt_array) opts = opt_array.map { |x| "'#{x}'" } - return opts.join(" and ") if opts.size < 3 - opts[0..-2].join(", ") + " and " + opts[-1] + return opts.join(" or ") if opts.size < 3 + opts[0..-2].join(", ") + ", or " + opts[-1] end def self.included(receiver) diff --git a/spec/mixlib/cli_spec.rb b/spec/mixlib/cli_spec.rb index 3adda52..4088f20 100644 --- a/spec/mixlib/cli_spec.rb +++ b/spec/mixlib/cli_spec.rb @@ -224,17 +224,25 @@ describe Mixlib::CLI do expect(@cli.config[:inclusion]).to eql("one") end - it "changes description if :in key is specified" do - TestCLI.option(:inclusion, short: "-i val", in: %w{one two}, description: "desc", required: false) + it "changes description if :in key is specified with a single value" do + TestCLI.option(:inclusion, short: "-i val", in: %w{one}, description: "desc", required: false) @cli = TestCLI.new @cli.parse_options(["-i", "one"]) - expect(@cli.options[:inclusion][:description]).to eql("desc (valid options: 'one' and 'two')") + expect(@cli.options[:inclusion][:description]).to eql("desc (valid options: 'one')") end - it "Raises SystemExit when the provided value is not a member of the :in array" do + it "changes description if :in key is specified with 2 values" do TestCLI.option(:inclusion, short: "-i val", in: %w{one two}, description: "desc", required: false) @cli = TestCLI.new - expect(lambda { @cli.parse_options(["-i", "three"]) }).to raise_error(SystemExit) + @cli.parse_options(["-i", "one"]) + expect(@cli.options[:inclusion][:description]).to eql("desc (valid options: 'one' or 'two')") + end + + it "changes description if :in key is specified with 3 values" do + TestCLI.option(:inclusion, short: "-i val", in: %w{one two three}, description: "desc", required: false) + @cli = TestCLI.new + @cli.parse_options(["-i", "one"]) + expect(@cli.options[:inclusion][:description]).to eql("desc (valid options: 'one', 'two', or 'three')") end it "doesn't exit if a required option is specified" do -- cgit v1.2.1