From e4f33ad8958387e5775a232f633f671df000b716 Mon Sep 17 00:00:00 2001 From: "Marc A. Paradise" Date: Tue, 4 Jun 2019 16:04:11 -0400 Subject: [MIXLIB-CLI-63] Add deprecated_option support This commit adds deprecated option support by exposing a new ClassMethod, `deprecated_option`. It will generate a corresponding deprecated option, and if that option is used it will handle mapping of the old option to the new and issue a warning. `deprecated_option` accepts a subset of the parameters that `option` accepts. Most importantly, a deprecated option can't have a default value. There's a practical reason for this and a philosophical one. Practically, it makes it easy to track the situation where someone has set `use_separate_defaults` to `false`. In that case, we currently can't tell whether the user provided the flag, or it was set as a default. This could have been addressed, but: Philosophically it makes more sense to not have a default value on a deprecated item. If it's deprecated, you want people to stop using it. If it has a default, it's effectively forced in-use at all times. See function docs for further accepted parameters. To allow deprecated options without warnings, use parse_options as `parse_options(ARGV, show_deprecations: false)`. By default, warnings will be shown. This also moves some formatting code into its own class - it was causing methods to get mixed in that client classes didn't need; and I reached the point where I needed to access the formatting functions across methods in both Mixlib::CLI and Mixlib::CLI::ClassMethods. It made more sense to move them outside of the mixed-in bits, since it wasn't a concern of the caller that would be inheriting those methods. Signed-off-by: Marc A. Paradise --- README.md | 134 +++++++++++++++++++++++++++++++---- lib/mixlib/cli.rb | 167 ++++++++++++++++++++++++++++++++++++-------- lib/mixlib/cli/formatter.rb | 29 ++++++++ spec/mixlib/cli_spec.rb | 134 +++++++++++++++++++++++++++++++++-- 4 files changed, 414 insertions(+), 50 deletions(-) create mode 100644 lib/mixlib/cli/formatter.rb diff --git a/README.md b/README.md index b6aa0af..b186d48 100644 --- a/README.md +++ b/README.md @@ -73,19 +73,8 @@ c.run MyConfig[:log_level] # :debug ``` -Available arguments to 'option': - -- `:short`: The short option, just like from optparse. Example: "-l LEVEL" -- `:long`: The long option, just like from optparse. Example: "--level LEVEL" -- `:description`: The description for this item, just like from optparse. -- `:default`: A default value for this option -- `:required`: Prints a message informing the user of the missing requirement, and exits. Default is false. -- `:on`: Set to :tail to appear at the end, or `:head`: to appear at the top. -- `:boolean:`: If this option takes no arguments, set this to true. -- `:show_options`: If you want the option list printed when this option is called, set this to true. -- `:exit`: Exit your program with the exit code when this option is specified. Example: 0 -- `:proc`: If set, the configuration value will be set to the return value of this proc. -- `:in`: An array containing the list of accepted values +For supported arguments to `option`, see the function documentation in [lib/mixlib/cli.rb](lib/mixlib/cli.rb). + If you need access to the leftover options that aren't captured in the config, you can get at them through +cli_arguments+ (referring to the above definition of MyCLI). @@ -96,14 +85,131 @@ cli.parse_options cli.cli_arguments # [ 'file1', 'file2', 'file3' ] ``` +## Deprecating CLI Options + +mixlib-cli 2.1.0 and later supports declaring options as deprecated. Using a deprecated option +will result in a warning message being displayed. If a deprecated flag is supplied, +its value is assigned to the replacement flag. You can control this assignment by specifying a +`value_mapper` function in the arguments (see example below, and function docs) + + +Usage notes (see docs for arguments to `Mixlib::CLI::ClassMethods#deprecated_option` for more): + + * Define deprecated items last, after all non-deprecated items have been defined. + You will see errors if your deprecated item references a `replacement` that hasn't been defined yet. + * deprecated\_option only takes a subset of 'option' configuration. You can only specify `short`, `long` - these should + map to the short/long values of the option from before its deprecation. + * item description will automatically be generated along the lines of "-f/--flag is deprecated. Use -n/--new-flag instead." + * if the `replacement` argument is not given, item description will look like "-f/--flag is deprecated and will be removed in a future release" + +### Example + +Given the following example: + +```ruby + +# mycli.rb + +class MyCLI + include Mixlib::CLI + + + option :arg_not_required, + description: "This takes no argument.", + long: "--arg-not-required", + short: "-n" + + option :arg_required, + description: "This takes an argument.", + long: "--arg-required ARG", + short: "-a", + in: ["a", "b", "c"] + + deprecated_option :dep_one, + short: "-1", + long: "--dep-one", + # this deprecated option maps to the '--arg-not-required' option: + replacement: :arg_not_required, + # Do not keep 'dep_one' in `config` after parsing. + keep: false + + deprecated_option :dep_two, + short: "-2", + long: "--dep-two ARG", + replacement: :arg_required, + # will map the given value to a valid value for `--arg-required`. + value_mapper: Proc.new { |arg| + case arg + when "q"; "invalid" # We'll use this to show validation still works + when "z"; "a" + when "x"; "b" + else + "c" + end + } + +end + +c = MyCLI.new() +c.parse_options + +puts "arg_required: #{c.config[:arg_required]}" if c.config.key? :arg_required +puts "arg_not_required: #{c.config[:arg_not_required]}" if c.config.key? :arg_not_required +puts "dep_one: #{c.config[:dep_one]}" if c.config.key? :dep_one +puts "dep_two: #{c.config[:dep_two]}" if c.config.key? :dep_two + +``` + +In this example, --dep-one will be used. Note that dep_one will not have a value of its own in +`options` because `keep: false` was given to the deprecated option. + +```bash + +$ ruby mycli.rb --dep-one + +-1/--dep-one: This flag is deprecated. Use -n/--arg-not-required instead +arg_not_required: true + +``` + +In this example, the value provided to dep-two will be converted to a value +that --arg-required will accept,a nd populate `:arg\_required` with + +```bash + +$ ruby mycli.rb --dep-two z # 'q' maps to 'invalid' in the value_mapper proc above + +-2/--dep-two: This flag is deprecated. Use -a/--arg-required instead. + +arg_required: a # The value is mapped to its replacement using the function provided. +dep_two: z # the deprected value is kept by default +``` + +In this example, the value provided to dep-two will be converted to a value +that --arg-required will reject, showing how content rules are applied even when +the input is coming from a deprecated option: + +```bash +$ ruby mycli.rb --dep-two q + +-2/--dep-two: This flag is deprecated. Use -a/--arg-required instead. +-a/--arg-required: invalid is not one of the allowed values: 'a', 'b', or 'c' + +``` ## Documentation -All documentation is written using YARD. You can generate a by running: +Class and module documentation is maintained using YARD. You can generate it by running: ``` rake docs ``` +You can serve them locally with live refresh using: + +``` +bundle exec yard server --reload +``` + ## Contributing For information on contributing to this project please see our [Contributing Documentation](https://github.com/chef/chef/blob/master/CONTRIBUTING.md) diff --git a/lib/mixlib/cli.rb b/lib/mixlib/cli.rb index f3598e6..9610bb4 100644 --- a/lib/mixlib/cli.rb +++ b/lib/mixlib/cli.rb @@ -17,7 +17,7 @@ # require "optparse" - +require "mixlib/cli/formatter" module Mixlib # == Mixlib::CLI @@ -30,8 +30,9 @@ module Mixlib # === DSL # When included, Mixlib::CLI also extends the including class with its # ClassMethods, which define the DSL. The primary methods of the DSL are - # ClassMethods#option, which defines a command line option, and - # ClassMethods#banner, which defines the "usage" banner. + # ClassMethods#option, which defines a command line option; + # ClassMethods#banner, which defines the "usage" banner; + # and ClassMethods#deprecated_option, which defines a deprecated command-line option. # # === Parsing # Command line options are parsed by calling the instance method @@ -93,14 +94,88 @@ module Mixlib # === Parameters # name:: The name of the option to add # args:: A hash of arguments for the option, specifying how it should be parsed. - # === Returns - # true:: Always returns true. + # Supported arguments: + # :short - The short option, just like from optparse. Example: "-l LEVEL" + # :long - The long option, just like from optparse. Example: "--level LEVEL" + # :description - The description for this item, just like from optparse. + # :default - A default value for this option. Default values will be populated + # on parse into `config` or `default_default`, depending `use_separate_defaults` + # :boolean - indicates the flag is a boolean. You can use this if the flag takes no arguments + # The config value will be set to 'true' if the flag is provided on the CLI and this + # argument is set to true. The config value will be set to false only + # if it has a default value of false + # :required - When set, the option is required. If the command is run without this option, + # it will print a message informing the user of the missing requirement, and exit. Default is false. + # :proc - Proc that will be invoked if the human has specified this option. + # Two forms are supported: + # Proc/1 - provided value is passed in. + # Proc/2 - first argument is provided value. Second is the cli flag option hash. + # Both versions return the value to be assigned to the option. + # :show_options - this option is designated as one that shows all supported options/help when invoked. + # :exit - exit your program with the exit code when this option is given. Example: 0 + # :in - array containing a list of valid values. The value provided at run-time for the option is + # validated against this. If it is not in the list, it will print a message and exit. + # :on :head OR :tail - force this option to display at the beginning or end of the + # option list, respectively + # = + # @return :: the config hash for the created option + # def option(name, args) @options ||= {} raise(ArgumentError, "Option name must be a symbol") unless name.kind_of?(Symbol) @options[name.to_sym] = args end + # Declare a deprecated option + # + # Add a deprecated command line option. + # + # name :: The name of the deprecated option + # replacement :: The name of the option that replaces this option. + # long :: The original long flag name, or flag name with argument, eg "--user USER" + # short :: The original short-form flag name, eg "-u USER" + # boolean :: true if this is a boolean flag, eg "--[no-]option". + # value_mapper :: a block that accepts the original value from the deprecated option, + # and converts it to a value suitable for the new option. + # If not provided, the value provided to the deprecated option will be + # assigned directly to the converted option. + # keep :: Defaults to true, this ensure sthat `options[:deprecated_flag]` is + # populated when the deprecated flag is used. If set to false, + # only the value in `replacement` will be set. Results undefined + # if no replacement is provided. You can use this to enforce the transition + # to non-deprecated keys in your code. + # + # === Returns + # :: The config hash for the created option. + def deprecated_option(name, + replacement: nil, + long: nil, + short: nil, + boolean: false, + value_mapper: nil, + keep: true) + + description = if replacement + replacement_cfg = options[replacement] + display_name = CLI::Formatter.combined_option_display_name(replacement_cfg[:short], replacement_cfg[:long]) + "This flag is deprecated. Use #{display_name} instead." + else + "This flag is deprecated and will be removed in a future release." + end + value_mapper ||= Proc.new { |v| v } + + option(name, + long: long, + short: short, + boolean: boolean, + description: description, + on: :tail, + deprecated: true, + keep: keep, + replacement: replacement, + value_mapper: value_mapper) + end + # Get the hash of current options. # # === Returns @@ -145,6 +220,13 @@ 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 @@ -209,7 +291,6 @@ module Mixlib config_opts[:show_options] ||= false config_opts[:exit] ||= nil config_opts[:in] ||= nil - if config_opts.key?(:default) defaults_container[config_key] = config_opts[:default] end @@ -225,25 +306,30 @@ module Mixlib # # === Returns # argv:: Returns any un-parsed elements. - def parse_options(argv = ARGV) + def parse_options(argv = ARGV, show_deprecations: true) argv = argv.dup opt_parser.parse!(argv) + # Do this before our custom validations, so that custom + # validations apply to any converted deprecation values; + # but after parse! so that everything is populated. + handle_deprecated_options(show_deprecations) # Deal with any required values - options.each do |opt_key, opt_value| - if opt_value[:required] && !config.key?(opt_key) - reqarg = opt_value[:short] || opt_value[:long] + options.each do |opt_key, opt_config| + if opt_config[:required] && !config.key?(opt_key) + reqarg = opt_config[:short] || opt_config[:long] puts "You must supply #{reqarg}!" puts @opt_parser exit 2 end - if opt_value[:in] - unless opt_value[:in].kind_of?(Array) + if opt_config[:in] + unless opt_config[:in].kind_of?(Array) raise(ArgumentError, "Options config key :in must receive an Array") 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 one of the allowed values: #{friendly_opt_list(opt_value[:in])}" + if config[opt_key] && !opt_config[:in].include?(config[opt_key]) + reqarg = Formatter.combined_option_display_name(opt_config[:short], opt_config[:long]) + puts "#{reqarg}: #{config[opt_key]} is not one of the allowed values: #{Formatter.friendly_opt_list(opt_config[:in])}" + # TODO - get rid of this. nobody wants to be spammed with a ton of information, particularly since we just told them the exact problem and how to fix it. puts @opt_parser exit 2 end @@ -267,7 +353,6 @@ module Mixlib # Create new options options.sort { |a, b| a[0].to_s <=> b[0].to_s }.each do |opt_key, opt_val| opt_args = build_option_arguments(opt_val) - opt_method = case opt_val[:on] when :on :on @@ -305,15 +390,48 @@ module Mixlib end end + # Iterates through options declared as deprecated, + # maps values to their replacement options, + # and prints deprecation warnings. + # + # @return NilClass + def handle_deprecated_options(show_deprecations) + config.keys.each 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' + # entry that contains a deprecated indicator, then the option was + # explicitly provided by the caller. + next unless opt_cfg[:deprecated] + replacement_key = opt_cfg[:replacement] + if replacement_key + # This is the value passed into the deprecated flag. We'll use + # 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) + config.delete(opt_key) unless opt_cfg[:keep] + end + + # Warn about the deprecation. + if show_deprecations + # Description is also the deprecation message. + display_name = CLI::Formatter.combined_option_display_name(opt_cfg[:short], opt_cfg[:long]) + puts "#{display_name}: #{opt_cfg[:description]}" + end + end + nil + end + def build_option_arguments(opt_setting) arguments = Array.new - arguments << opt_setting[:short] if opt_setting.key?(:short) - arguments << opt_setting[:long] if opt_setting.key?(:long) + arguments << opt_setting[:short] unless opt_setting[:short].nil? + arguments << opt_setting[:long] unless opt_setting[:long].nil? if opt_setting.key?(:description) description = opt_setting[:description].dup description << " (required)" if opt_setting[:required] - description << " (valid options: #{friendly_opt_list(opt_setting[:in])})" if opt_setting[:in] + description << " (valid options: #{Formatter.friendly_opt_list(opt_setting[:in])})" if opt_setting[:in] opt_setting[:description] = description arguments << description end @@ -321,20 +439,9 @@ module Mixlib arguments end - # @private - # @param opt_arry [Array] - # - # @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(" or ") if opts.size < 3 - opts[0..-2].join(", ") + ", or " + opts[-1] - end - def self.included(receiver) receiver.extend(Mixlib::CLI::ClassMethods) receiver.extend(Mixlib::CLI::InheritMethods) end - end end diff --git a/lib/mixlib/cli/formatter.rb b/lib/mixlib/cli/formatter.rb new file mode 100644 index 0000000..b89090d --- /dev/null +++ b/lib/mixlib/cli/formatter.rb @@ -0,0 +1,29 @@ + +module Mixlib + module CLI + class Formatter + # Create a string that includes both versions (short/long) of a flag name + # based on on whether short/long/both/neither are provided + # + # @param short [String] the short name of the option. Can be nil. + # @param long [String] the long name of the option. Can be nil. + # @return [String] the formatted flag name as described above + def self.combined_option_display_name(short, long) + usage = "" + usage << short.split(" ").first if short + usage << "/" if long && short + usage << long.split(" ").first if long + usage + end + + # @param opt_arry [Array] + # + # @return [String] a friendly quoted list of items complete with "or" + def self.friendly_opt_list(opt_array) + opts = opt_array.map { |x| "'#{x}'" } + return opts.join(" or ") if opts.size < 3 + opts[0..-2].join(", ") + ", or " + opts[-1] + end + end + end +end diff --git a/spec/mixlib/cli_spec.rb b/spec/mixlib/cli_spec.rb index 4088f20..acc10f8 100644 --- a/spec/mixlib/cli_spec.rb +++ b/spec/mixlib/cli_spec.rb @@ -31,11 +31,30 @@ describe Mixlib::CLI do end end + describe "deprecated_option" do + it "makes a deprecated option when you declare one" do + TestCLI.deprecated_option(:option_d, short: "-d") + expect(TestCLI.options[:option_d]).to include(deprecated: true) + end + end + describe "options" do it "returns the current options hash" do TestCLI.option(:config_file, short: "-c CONFIG") expect(TestCLI.options).to eql({ config_file: { short: "-c CONFIG" } }) end + it "includes deprecated options and their generated descriptions" do + TestCLI.option(:config_file, short: "-c CONFIG") + TestCLI.deprecated_option(:blah, short: "-b BLAH") + TestCLI.deprecated_option(:blah2, long: "--blah2 BLAH", replacement: :config_file) + opts = TestCLI.options + expect(opts[:config_file][:short]).to eql("-c CONFIG") + expect(opts[:config_file].key?(:deprecated)).to eql(false) + expect(opts[:blah][:description]).to eql("This flag is deprecated and will be removed in a future release.") + expect(opts[:blah][:deprecated]).to eql(true) + expect(opts[:blah2][:description]).to eql("This flag is deprecated. Use -c instead.") + expect(opts[:blah2][:deprecated]).to eql(true) + end end describe "options=" do @@ -68,12 +87,12 @@ describe Mixlib::CLI do expect(@cli.banner).to eql(TestCLI.banner) end - it "sets the options to the class defined options, plus defaults" do - TestCLI.option(:config_file, short: "-l LOG") + it "sets the options to the class defined options and deprecated options, with defaults" do + TestCLI.option(:config_file, short: "-l FILE") + TestCLI.deprecated_option(:option_file, short: "-o FILE", replacement: :config_file) cli = TestCLI.new - expect(cli.options).to eql({ - config_file: { - short: "-l LOG", + expect(cli.options[:config_file]).to eql({ + short: "-l FILE", on: :on, boolean: false, required: false, @@ -81,8 +100,24 @@ describe Mixlib::CLI do show_options: false, exit: nil, in: nil, - }, }) + + expect(cli.options[:option_file]).to include( + boolean: false, + deprecated: true, + description: "This flag is deprecated. Use -l instead.", + exit: nil, + in: nil, + long: nil, + keep: true, + proc: nil, + replacement: :config_file, + required: false, + short: "-o FILE", + on: :tail, + show_options: false + ) + expect(cli.options[:option_file][:value_mapper].class).to eql(Proc) end it "sets the default config value for any options that include it" do @@ -282,6 +317,93 @@ describe Mixlib::CLI do expect(@cli.parse_options([ "easy", "-p", "opscode", "hard" ])).to eql(%w{easy hard}) expect(@cli.cli_arguments).to eql(%w{easy hard}) end + + describe "with non-deprecated and deprecated options" do + let(:cli) { TestCLI.new } + before do + TestCLI.option(:option_a, long: "--[no-]option-a", boolean: true) + TestCLI.option(:option_b, short: "-b ARG", in: %w{a b c}) + TestCLI.option(:option_c, short: "-c ARG") + end + + context "when the deprecated option has a replacement" do + + context "and a value_mapper is provided" do + before do + TestCLI.deprecated_option(:option_x, + long: "--option-x ARG", + replacement: :option_b, + value_mapper: Proc.new { |val| val == "valid" ? "a" : "xxx" } ) + end + + it "still checks the replacement's 'in' validation list" do + expect { cli.parse_options(%w{--option-x invalid}) }.to raise_error SystemExit + end + + it "sets the mapped value in the replacement option and the deprecated value in the deprecated option" do + cli.parse_options(%w{--option-x valid}) + expect(cli.config[:option_x]).to eql("valid") + expect(cli.config[:option_b]).to eql("a") + end + end + + context "and a value_mapper is not provided" do + context "and keep is set to false in the deprecated option" do + before do + TestCLI.deprecated_option(:option_x, + long: "--option-x ARG", + replacement: :option_c, + keep: false) + end + it "captures the replacement value, but does not set the deprecated value" do + cli.parse_options %w{--option-x blah} + expect(cli.config.key?(:option_x)).to eql false + expect(cli.config[:option_c]).to eql "blah" + end + end + + context "and the replacement and deprecated are both boolean" do + before do + TestCLI.deprecated_option(:option_x, boolean: true, + long: "--[no-]option-x", + replacement: :option_a) + end + it "sets original and replacement to true when the deprecated flag is used" do + cli.parse_options(%w{--option-x}) + expect(cli.config[:option_x]).to eql true + expect(cli.config[:option_a]).to eql true + end + it "sets the original and replacement to false when the negative deprecated flag is used" do + cli.parse_options(%w{--no-option-x}) + expect(cli.config[:option_x]).to eql false + expect(cli.config[:option_a]).to eql false + end + end + + context "when the replacement does not accept a value" do + before do + TestCLI.deprecated_option(:option_x, + long: "--option-x ARG", + replacement: :option_c) + end + + it "will still set the value because you haven't given a custom value mapper to set a true/false value" do + cli.parse_options(%w{--option-x BLAH}) + expect(cli.config[:option_c]).to eql("BLAH") + end + end + end + end + + context "when the deprecated option does not have a replacement" do + before do + TestCLI.deprecated_option(:option_x, short: "-x") + end + it "warns about the deprecated option being removed" do + expect { TestCLI.new.parse_options(%w{-x}) }.to output(/removed in a future release/).to_stdout + end + end + end end end -- cgit v1.2.1