+1 as a pair-er.
On Wed, Sep 22, 2010 at 2:35 PM, Paul Berry <[email protected]> wrote:
> The global "-o" option ("--onetime") was overriding the
> application-specific option "-o" because global options were being
> sent to the OptionParser after application-specific options.
>
> Modified the order in which options are sent to the OptionParser to
> have the correct behavior. Also merged together the two methods that
> were applying options so that the order is more explicit.
>
> Signed-off-by: Paul Berry <[email protected]>
> ---
> This patch must be applied on top of "[#4798] Puppet doc manifests
> documentation mode broken", which I sent to the list yesterday.
>
> Available at http://github.com/stereotype441/puppet/tree/ticket/2.6.x/4822
>
> lib/puppet/application.rb | 38
> ++++++++++++---------------
> spec/integration/application/doc_spec.rb | 7 +++++
> spec/unit/application_spec.rb | 41
> +++++++++++------------------
> 3 files changed, 40 insertions(+), 46 deletions(-)
>
> diff --git a/lib/puppet/application.rb b/lib/puppet/application.rb
> index 2fec38b..f0159a6 100644
> --- a/lib/puppet/application.rb
> +++ b/lib/puppet/application.rb
> @@ -254,19 +254,6 @@ class Application
> def preinit
> end
>
> - def option_parser
> - return @option_parser if defined?(@option_parser)
> -
> - @option_parser = OptionParser.new(self.class.banner)
> -
> - self.class.option_parser_commands.each do |options, fname|
> - @option_parser.on(*options) do |value|
> - self.send(fname, value)
> - end
> - end
> - @option_parser
> - end
> -
> def initialize(command_line = nil)
> require 'puppet/util/command_line'
> @command_line = command_line || Puppet::Util::CommandLine.new
> @@ -323,20 +310,29 @@ class Application
> end
>
> def parse_options
> - # get all puppet options
> - optparse_opt = []
> - optparse_opt = Puppet.settings.optparse_addargs(optparse_opt)
> + # Create an option parser
> + option_parser = OptionParser.new(self.class.banner)
>
> - # convert them to OptionParser format
> - optparse_opt.each do |option|
> - self.option_parser.on(*option) do |arg|
> + # Add all global options to it.
> + Puppet.settings.optparse_addargs([]).each do |option|
> + option_parser.on(*option) do |arg|
> handlearg(option[0], arg)
> end
> end
>
> - # scan command line argument
> + # Add options that are local to this application, which were
> + # created using the "option()" metaprogramming method. If there
> + # are any conflicts, this application's options will be favored.
> + self.class.option_parser_commands.each do |options, fname|
> + option_parser.on(*options) do |value|
> + # Call the method that "option()" created.
> + self.send(fname, value)
> + end
> + end
> +
> + # scan command line.
> begin
> - self.option_parser.parse!(self.command_line.args)
> + option_parser.parse!(self.command_line.args)
> rescue OptionParser::ParseError => detail
> $stderr.puts detail
> $stderr.puts "Try 'puppet #{command_line.subcommand_name} --help'"
> diff --git a/spec/integration/application/doc_spec.rb
> b/spec/integration/application/doc_spec.rb
> index cb9f478..eaf5442 100644
> --- a/spec/integration/application/doc_spec.rb
> +++ b/spec/integration/application/doc_spec.rb
> @@ -45,4 +45,11 @@ describe Puppet::Application::Doc do
> Dir.chdir(old_dir)
> end
> end
> +
> + it "should respect the -o option" do
> + puppetdoc = Puppet::Application[:doc]
> + puppetdoc.command_line.stubs(:args).returns(['foo', '-o', 'bar'])
> + puppetdoc.parse_options
> + puppetdoc.options[:outputdir].should == 'bar'
> + end
> end
> diff --git a/spec/unit/application_spec.rb b/spec/unit/application_spec.rb
> index 4338091..be7cda3 100755
> --- a/spec/unit/application_spec.rb
> +++ b/spec/unit/application_spec.rb
> @@ -191,24 +191,17 @@ describe Puppet::Application do
> Puppet.settings.stubs(:optparse_addargs).returns([])
> end
>
> - it "should create a new option parser when needed" do
> - option_parser = stub "option parser"
> - option_parser.stubs(:on)
> - OptionParser.expects(:new).returns(option_parser).once
> - @app.option_parser.should == option_parser
> - @app.option_parser.should == option_parser
> - end
> -
> it "should pass the banner to the option parser" do
> option_parser = stub "option parser"
> option_parser.stubs(:on)
> + option_parser.stubs(:parse!)
> @app.class.instance_eval do
> banner "banner"
> end
>
> OptionParser.expects(:new).with("banner").returns(option_parser)
>
> - @app.option_parser
> + @app.parse_options
> end
>
> it "should get options from Puppet.settings.optparse_addargs" do
> @@ -219,15 +212,14 @@ describe Puppet::Application do
>
> it "should add Puppet.settings options to OptionParser" do
> Puppet.settings.stubs(:optparse_addargs).returns( [["--option","-o",
> "Funny Option"]])
> -
> - @app.option_parser.expects(:on).with { |*arg| arg ==
> ["--option","-o", "Funny Option"] }
> -
> + Puppet.settings.expects(:handlearg).with("--option", 'true')
> + @app.command_line.stubs(:args).returns(["--option"])
> @app.parse_options
> end
>
> it "should ask OptionParser to parse the command-line argument" do
> @app.command_line.stubs(:args).returns(%w{ fake args })
> - @app.option_parser.expects(:parse!).with(%w{ fake args })
> + OptionParser.any_instance.expects(:parse!).with(%w{ fake args })
>
> @app.parse_options
> end
> @@ -258,31 +250,30 @@ describe Puppet::Application do
>
> describe "when dealing with an argument not declared directly by the
> application" do
> it "should pass it to handle_unknown if this method exists" do
> -
> Puppet.settings.stubs(:optparse_addargs).returns([["--not-handled"]])
> - @app.option_parser.stubs(:on).yields("value")
> +
> Puppet.settings.stubs(:optparse_addargs).returns([["--not-handled",
> :REQUIRED]])
>
> @app.expects(:handle_unknown).with("--not-handled",
> "value").returns(true)
> -
> + @app.command_line.stubs(:args).returns(["--not-handled", "value"])
> @app.parse_options
> end
>
> it "should pass it to Puppet.settings if handle_unknown says so" do
> - Puppet.settings.stubs(:optparse_addargs).returns([["--topuppet"]])
> - @app.option_parser.stubs(:on).yields("value")
> + Puppet.settings.stubs(:optparse_addargs).returns([["--topuppet",
> :REQUIRED]])
>
> @app.stubs(:handle_unknown).with("--topuppet",
> "value").returns(false)
>
> Puppet.settings.expects(:handlearg).with("--topuppet", "value")
> + @app.command_line.stubs(:args).returns(["--topuppet", "value"])
> @app.parse_options
> end
>
> it "should pass it to Puppet.settings if there is no handle_unknown
> method" do
> - Puppet.settings.stubs(:optparse_addargs).returns([["--topuppet"]])
> - @app.option_parser.stubs(:on).yields("value")
> + Puppet.settings.stubs(:optparse_addargs).returns([["--topuppet",
> :REQUIRED]])
>
> @app.stubs(:respond_to?).returns(false)
>
> Puppet.settings.expects(:handlearg).with("--topuppet", "value")
> + @app.command_line.stubs(:args).returns(["--topuppet", "value"])
> @app.parse_options
> end
>
> @@ -310,7 +301,7 @@ describe Puppet::Application do
>
> it "should exit if OptionParser raises an error" do
> $stderr.stubs(:puts)
> -
> @app.option_parser.stubs(:parse!).raises(OptionParser::ParseError.new("blah
> blah"))
> +
>
> OptionParser.any_instance.stubs(:parse!).raises(OptionParser::ParseError.new("blah
> blah"))
>
> @app.expects(:exit)
>
> @@ -478,7 +469,7 @@ describe Puppet::Application do
> @app.class.option("--[no-]test3","-t") do
> end
>
> - @app.option_parser
> + @app.parse_options
> end
>
> it "should pass a block that calls our defined method" do
> @@ -490,7 +481,7 @@ describe Puppet::Application do
> @app.class.option("--test4","-t") do
> end
>
> - @app.option_parser
> + @app.parse_options
> end
> end
>
> @@ -501,7 +492,7 @@ describe Puppet::Application do
>
> @app.class.option("--test4","-t")
>
> - @app.option_parser
> + @app.parse_options
> end
>
> it "should give to OptionParser a block that adds the the value to
> the options array" do
> @@ -512,7 +503,7 @@ describe Puppet::Application do
>
> @app.class.option("--test4","-t")
>
> - @app.option_parser
> + @app.parse_options
> end
> end
> end
> --
> 1.7.2
>
> --
> You received this message because you are subscribed to the Google Groups
> "Puppet Developers" group.
> To post to this group, send email to [email protected].
> To unsubscribe from this group, send email to
> [email protected]<puppet-dev%[email protected]>
> .
> For more options, visit this group at
> http://groups.google.com/group/puppet-dev?hl=en.
>
>
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
