[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, experimental, updated. debian/2.6.8-1-844-g7ec39d5
Daniel Pittman
daniel at puppetlabs.com
Tue May 10 08:10:40 UTC 2011
The following commit has been merged in the experimental branch:
commit 423fe1f6b7c5bc0ca9b53a87f636668514802ad7
Author: Daniel Pittman <daniel at puppetlabs.com>
Date: Wed Mar 30 16:58:17 2011 -0700
(#6749) string cli base: implement preinit CLI parsing
In order to identify the full set of options we need to know the action that
is being invoked; that actually requires a pre-processing step to identify
that out of the global options.
Notably, our spec is that options can be to the right of their declaration
point, but not to the left: that means that we can now extract the full set of
options, and interact with the standard Puppet option handling code, resulting
in at least vaguely saner behaviour...
Reviewed-By: Pieter van de Bruggen <pieter at puppetlabs.com>
diff --git a/lib/puppet/application/string_base.rb b/lib/puppet/application/string_base.rb
index ffd49e8..1169a45 100644
--- a/lib/puppet/application/string_base.rb
+++ b/lib/puppet/application/string_base.rb
@@ -5,14 +5,6 @@ class Puppet::Application::StringBase < Puppet::Application
should_parse_config
run_mode :agent
- def preinit
- super
- trap(:INT) do
- $stderr.puts "Cancelling String"
- exit(0)
- end
- end
-
option("--debug", "-d") do |arg|
Puppet::Util::Log.level = :debug
end
@@ -32,7 +24,7 @@ class Puppet::Application::StringBase < Puppet::Application
end
- attr_accessor :string, :type, :verb, :arguments, :format
+ attr_accessor :string, :action, :type, :arguments, :format
attr_writer :exit_code
# This allows you to set the exit code if you don't want to just exit
@@ -41,14 +33,6 @@ class Puppet::Application::StringBase < Puppet::Application
@exit_code || 0
end
- def main
- # Call the method associated with the provided action (e.g., 'find').
- if result = string.send(verb, *arguments)
- puts render(result)
- end
- exit(exit_code)
- end
-
# Override this if you need custom rendering.
def render(result)
render_method = Puppet::Network::FormatHandler.format(format).render_method
@@ -61,16 +45,14 @@ class Puppet::Application::StringBase < Puppet::Application
end
def preinit
+ super
+ trap(:INT) do
+ $stderr.puts "Cancelling String"
+ exit(0)
+ end
+
# We need to parse enough of the command line out early, to identify what
# the action is, so that we can obtain the full set of options to parse.
- #
- # This requires a partial parse first, and removing the options that we
- # understood, then identification of the next item, then another round of
- # the same until we have the string and action all set. --daniel 2011-03-29
- #
- # NOTE: We can't use the Puppet::Application implementation of option
- # parsing because it is (*ahem*) going to puts on $stderr and exit when it
- # hits a parse problem, not actually let us reuse stuff. --daniel 2011-03-29
# TODO: These should be configurable versions, through a global
# '--version' option, but we don't implement that yet... --daniel 2011-03-29
@@ -78,19 +60,42 @@ class Puppet::Application::StringBase < Puppet::Application
@string = Puppet::String[@type, :current]
@format = @string.default_format
- # Now, collect the global and string options and parse the command line.
- begin
- @string.options.inject OptionParser.new do |options, option|
- option = @string.get_option option # turn it into the object, bleh
- options.on(*option.to_optparse) do |value|
- puts "REVISIT: do something with #{value.inspect}"
+ # Now, walk the command line and identify the action. We skip over
+ # arguments based on introspecting the action and all, and find the first
+ # non-option word to use as the action.
+ action = nil
+ cli = command_line.args.dup # we destroy this copy, but...
+ while @action.nil? and not cli.empty? do
+ item = cli.shift
+ if item =~ /^-/ then
+ option = @string.options.find { |a| item =~ /^-+#{a}\b/ }
+ if option then
+ if @string.get_option(option).takes_argument? then
+ # We don't validate if the argument is optional or mandatory,
+ # because it doesn't matter here. We just assume that errors will
+ # be caught later. --daniel 2011-03-30
+ cli.shift unless cli.first =~ /^-/
+ end
+ else
+ raise ArgumentError, "Unknown option #{item.sub(/=.*$/, '').inspect}"
end
- end.parse! command_line.args.dup
- rescue OptionParser::InvalidOption => e
- puts e.inspect # ...and ignore??
+ else
+ action = @string.get_action(item.to_sym)
+ if action.nil? then
+ raise ArgumentError, "#{@string} does not have an #{item.inspect} action!"
+ end
+ @action = action
+ end
end
- fail "REVISIT: Finish this code, eh..."
+ @action or raise ArgumentError, "No action given on the command line!"
+
+ # Finally, we can interact with the default option code to build behaviour
+ # around the full set of options we now know we support.
+ @action.options.each do |option|
+ option = @action.get_option(option) # make it the object.
+ self.class.option(*option.optparse) # ...and make the CLI parse it.
+ end
end
def setup
@@ -100,18 +105,21 @@ class Puppet::Application::StringBase < Puppet::Application
# action to read in the options. This replaces the older model where we
# would invoke the action with options set as global state in the
# interface object. --daniel 2011-03-28
- @verb = command_line.args.shift
@arguments = Array(command_line.args) << options
validate
end
+
+ def main
+ # Call the method associated with the provided action (e.g., 'find').
+ if result = @string.send(@action.name, *arguments)
+ puts render(result)
+ end
+ exit(exit_code)
+ end
def validate
- unless verb
+ unless @action
raise "You must specify #{string.actions.join(", ")} as a verb; 'save' probably does not work right now"
end
-
- unless string.action?(verb)
- raise "Command '#{verb}' not found for #{type}"
- end
end
end
diff --git a/lib/puppet/string/option.rb b/lib/puppet/string/option.rb
index 70d62a0..e7b6f18 100644
--- a/lib/puppet/string/option.rb
+++ b/lib/puppet/string/option.rb
@@ -2,6 +2,7 @@ class Puppet::String::Option
attr_reader :parent
attr_reader :name
attr_reader :aliases
+ attr_reader :optparse
attr_accessor :desc
def takes_argument?
diff --git a/spec/unit/application/string_base_spec.rb b/spec/unit/application/string_base_spec.rb
index 65cadb8..1072d9b 100755
--- a/spec/unit/application/string_base_spec.rb
+++ b/spec/unit/application/string_base_spec.rb
@@ -23,11 +23,72 @@ describe Puppet::Application::StringBase do
$LOAD_PATH.pop
end
- before do
- @app = Puppet::Application::StringBase::Basetest.new
- @app.stubs(:exit)
- @app.stubs(:puts)
+ let :app do
+ app = Puppet::Application::StringBase::Basetest.new
+ app.stubs(:exit)
+ app.stubs(:puts)
+ app.command_line.stubs(:subcommand_name).returns 'subcommand'
+ app.command_line.stubs(:args).returns []
Puppet::Util::Log.stubs(:newdestination)
+ app
+ end
+
+ describe "#preinit" do
+ before :each do
+ app.command_line.stubs(:args).returns %w{}
+ end
+
+ it "should set the string based on the type"
+ it "should set the format based on the string default"
+
+ describe "parsing the command line" do
+ before :all do
+ Puppet::String[:basetest, '0.0.1'].action :foo do
+ option "--foo"
+ invoke do |options|
+ options
+ end
+ end
+ end
+
+ it "should find the action" do
+ app.command_line.stubs(:args).returns %w{foo}
+ app.preinit
+ app.action.should be
+ app.action.name.should == :foo
+ end
+
+ it "should fail if no action is given" do
+ expect { app.preinit }.
+ should raise_error ArgumentError, /No action given/
+ end
+
+ it "should report a sensible error when options with = fail" do
+ app.command_line.stubs(:args).returns %w{--foo=bar foo}
+ expect { app.preinit }.
+ should raise_error ArgumentError, /Unknown option "--foo"/
+ end
+
+ it "should fail if an action option is before the action" do
+ app.command_line.stubs(:args).returns %w{--foo foo}
+ expect { app.preinit }.
+ should raise_error ArgumentError, /Unknown option "--foo"/
+ end
+
+ it "should fail if an unknown option is before the action" do
+ app.command_line.stubs(:args).returns %w{--bar foo}
+ expect { app.preinit }.
+ should raise_error ArgumentError, /Unknown option "--bar"/
+ end
+
+ it "should not fail if an unknown option is after the action" do
+ app.command_line.stubs(:args).returns %w{foo --bar}
+ app.preinit
+ app.action.name.should == :foo
+ app.string.should_not be_option :bar
+ app.action.should_not be_option :bar
+ end
+ end
end
describe "when calling main" do
@@ -39,8 +100,7 @@ describe Puppet::Application::StringBase do
it "should send the specified verb and name to the string" do
@app.string.expects(:find).with("myname", "myarg")
-
- @app.main
+ app.main
end
it "should use its render method to render any result"
@@ -50,8 +110,8 @@ describe Puppet::Application::StringBase do
describe "during setup" do
before do
- @app.command_line.stubs(:args).returns(["find", "myname", "myarg"])
- @app.stubs(:validate)
+ app.command_line.stubs(:args).returns(["find", "myname", "myarg"])
+ app.stubs(:validate)
end
it "should set the verb from the command line arguments" do
diff --git a/spec/unit/string/action_spec.rb b/spec/unit/string/action_spec.rb
index 258ad5a..e5fefdb 100755
--- a/spec/unit/string/action_spec.rb
+++ b/spec/unit/string/action_spec.rb
@@ -62,6 +62,28 @@ describe Puppet::String::Action do
string.baz.should == "the value of foo in baz is '25'"
string.qux.should == "the value of foo in baz is '25'"
end
+
+ context "when calling the Ruby API" do
+ let :string do
+ Puppet::String.new(:ruby_api, '1.0.0') do
+ action :bar do
+ invoke do |options|
+ options
+ end
+ end
+ end
+ end
+
+ it "should work when no options are supplied" do
+ options = string.bar
+ options.should == {}
+ end
+
+ it "should work when options are supplied" do
+ options = string.bar :bar => "beer"
+ options.should == { :bar => "beer" }
+ end
+ end
end
describe "with action-level options" do
--
Puppet packaging for Debian
More information about the Pkg-puppet-devel
mailing list