Merge branch 'feature/master/6749-actions-need-to-support-options'

19 files changed, 403 insertions, 104 deletions

diff --git a/lib/puppet/application/configurer.rb b/lib/puppet/application/configurer.rb
index b440098ee..be018338f 100644
--- a/lib/puppet/application/configurer.rb
+++ b/lib/puppet/application/configurer.rb
@@ -5,8 +5,8 @@ class Puppet::Application::Configurer < Puppet::Application
   should_parse_config
   run_mode :agent
 
-  option("--debug","-d")
-  option("--verbose","-v")
+  option("--debug", "-d")
+  option("--verbose", "-v")
 
   def setup
     if options[:debug] or options[:verbose]
diff --git a/lib/puppet/application/indirection_base.rb b/lib/puppet/application/indirection_base.rb
index da61f408d..cfa1ea529 100644
--- a/lib/puppet/application/indirection_base.rb
+++ b/lib/puppet/application/indirection_base.rb
@@ -1,19 +1,4 @@
 require 'puppet/application/string_base'
 
 class Puppet::Application::IndirectionBase < Puppet::Application::StringBase
-  option("--terminus TERMINUS") do |arg|
-    @terminus = arg
-  end
-
-  attr_accessor :terminus, :indirection
-
-  def setup
-    super
-
-    if string.respond_to?(:indirection)
-      raise "Could not find data type #{type} for application #{self.class.name}" unless string.indirection
-
-      string.set_terminus(terminus) if terminus
-    end
-  end
 end
diff --git a/lib/puppet/application/string.rb b/lib/puppet/application/string.rb
index aa369e669..0a6a798ce 100644
--- a/lib/puppet/application/string.rb
+++ b/lib/puppet/application/string.rb
@@ -83,7 +83,7 @@ class Puppet::Application::String < Puppet::Application
   def actions(indirection)
     return [] unless string = Puppet::String[indirection, '0.0.1']
     string.load_actions
-    return string.actions.sort { |a,b| a.to_s <=> b.to_s }
+    return string.actions.sort { |a, b| a.to_s <=> b.to_s }
   end
 
   def load_applications
diff --git a/lib/puppet/application/string_base.rb b/lib/puppet/application/string_base.rb
index bc627adde..06e5789be 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
@@ -60,38 +44,85 @@ class Puppet::Application::StringBase < Puppet::Application
     end
   end
 
-  def setup
-    Puppet::Util::Log.newdestination :console
+  def preinit
+    super
+    trap(:INT) do
+      $stderr.puts "Cancelling String"
+      exit(0)
+    end
 
-    @verb = command_line.args.shift
-    @arguments = command_line.args
-    @arguments ||= []
+    # 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.
 
-    @arguments = Array(@arguments)
+    # TODO: These should be configurable versions, through a global
+    # '--version' option, but we don't implement that yet... --daniel 2011-03-29
+    @type   = self.class.name.to_s.sub(/.+:/, '').downcase.to_sym
+    @string = Puppet::String[@type, :current]
+    @format = @string.default_format
+
+    # 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
+    index  = -1
+    until @action or (index += 1) >= command_line.args.length do
+      item = command_line.args[index]
+      if item =~ /^-/ then
+        option = @string.options.find { |a| item =~ /^-+#{a}\b/ }
+        if option then
+          option = @string.get_option(option)
+          # If we have an inline argument, just carry on.  We don't need to
+          # care about optional vs mandatory in that case because we do a real
+          # parse later, and that will totally take care of raising the error
+          # when we get there. --daniel 2011-04-04
+          if option.takes_argument? and !item.index('=') then
+            index += 1 unless
+              (option.optional_argument? and command_line.args[index + 1] =~ /^-/)
+          end
+        else
+          raise ArgumentError, "Unknown option #{item.sub(/=.*$/, '').inspect}"
+        end
+      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
 
-    @type = self.class.name.to_s.sub(/.+:/, '').downcase.to_sym
+    @action or raise ArgumentError, "No action given on the command line!"
 
-    # TODO: These should be configurable versions.
-    unless Puppet::String.string?(@type, :current)
-      raise "Could not find any version of string '#{@type}'"
+    # 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
-    @string = Puppet::String[@type, :current]
-    @format ||= @string.default_format
+  end
 
-    # We copy all of the app options to the string.
-    # This allows each action to read in the options.
-    @string.options = options
+  def setup
+    Puppet::Util::Log.newdestination :console
 
-    validate
+    # We copy all of the app options to the end of the call; This allows each
+    # 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
+    #
+    # Note: because of our definition of where the action is set, we end up
+    # with it *always* being the first word of the remaining set of command
+    # line arguments.  So, strip that off when we construct the arguments to
+    # pass down to the string action. --daniel 2011-04-04
+    @arguments = command_line.args[1, -1] || []
+    @arguments << options
   end
 
-  def validate
-    unless verb
-      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}"
+  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
 end
diff --git a/lib/puppet/string.rb b/lib/puppet/string.rb
index 783b6afe0..517cf4506 100644
--- a/lib/puppet/string.rb
+++ b/lib/puppet/string.rb
@@ -2,12 +2,16 @@ require 'puppet'
 require 'puppet/util/autoload'
 
 class Puppet::String
-  require 'puppet/string/action_manager'
   require 'puppet/string/string_collection'
 
+  require 'puppet/string/action_manager'
   include Puppet::String::ActionManager
   extend Puppet::String::ActionManager
 
+  require 'puppet/string/option_manager'
+  include Puppet::String::OptionManager
+  extend Puppet::String::OptionManager
+
   include Puppet::Util
 
   class << self
@@ -53,12 +57,12 @@ class Puppet::String
     self.default_format = format.to_sym
   end
 
-  attr_accessor :type, :verb, :version, :arguments, :options
+  attr_accessor :type, :verb, :version, :arguments
   attr_reader :name
 
   def initialize(name, version, &block)
     unless Puppet::String::StringCollection.validate_version(version)
-      raise ArgumentError, "Cannot create string with invalid version number '#{version}'!"
+      raise ArgumentError, "Cannot create string #{name.inspect} with invalid version number '#{version}'!"
     end
 
     @name = Puppet::String::StringCollection.underscorize(name)
diff --git a/lib/puppet/string/action.rb b/lib/puppet/string/action.rb
index 4db9e97e2..ee3b2991b 100644
--- a/lib/puppet/string/action.rb
+++ b/lib/puppet/string/action.rb
@@ -1,26 +1,121 @@
+# -*- coding: utf-8 -*-
 require 'puppet/string'
+require 'puppet/string/option'
 
 class Puppet::String::Action
   attr_reader :name
 
-  def initialize(string, name, attrs = {})
-    name = name.to_s
-    raise "'#{name}' is an invalid action name" unless name =~ /^[a-z]\w*$/
-
-    @string = string
-    @name      = name
-    attrs.each do |k,v| send("#{k}=", v) end
+  def to_s
+    "#{@string}##{@name}"
   end
 
-  def invoke(*args, &block)
-    @string.method(name).call(*args,&block)
+  def initialize(string, name, attrs = {})
+    raise "#{name.inspect} is an invalid action name" unless name.to_s =~ /^[a-z]\w*$/
+    @string  = string
+    @name    = name.to_sym
+    @options = {}
+    attrs.each do |k, v| send("#{k}=", v) end
   end
 
+  # Initially, this was defined to allow the @action.invoke pattern, which is
+  # a very natural way to invoke behaviour given our introspection
+  # capabilities.   Heck, our initial plan was to have the string delegate to
+  # the action object for invocation and all.
+  #
+  # It turns out that we have a binding problem to solve: @string was bound to
+  # the parent class, not the subclass instance, and we don't pass the
+  # appropriate context or change the binding enough to make this work.
+  #
+  # We could hack around it, by either mandating that you pass the context in
+  # to invoke, or try to get the binding right, but that has probably got
+  # subtleties that we don't instantly think of – especially around threads.
+  #
+  # So, we are pulling this method for now, and will return it to life when we
+  # have the time to resolve the problem.  For now, you should replace...
+  #
+  #     @action = @string.get_action(name)
+  #     @action.invoke(arg1, arg2, arg3)
+  #
+  # ...with...
+  #
+  #     @action = @string.get_action(name)
+  #     @string.send(@action.name, arg1, arg2, arg3)
+  #
+  # I understand that is somewhat cumbersome, but it functions as desired.
+  # --daniel 2011-03-31
+  #
+  # PS: This code is left present, but commented, to support this chunk of
+  # documentation, for the benefit of the reader.
+  #
+  # def invoke(*args, &block)
+  #   @string.send(name, *args, &block)
+  # end
+
   def invoke=(block)
+    # We need to build an instance method as a wrapper, using normal code, to
+    # be able to expose argument defaulting between the caller and definer in
+    # the Ruby API.  An extra method is, sadly, required for Ruby 1.8 to work.
+    #
+    # In future this also gives us a place to hook in additional behaviour
+    # such as calling out to the action instance to validate and coerce
+    # parameters, which avoids any exciting context switching and all.
+    #
+    # Hopefully we can improve this when we finally shuffle off the last of
+    # Ruby 1.8 support, but that looks to be a few "enterprise" release eras
+    # away, so we are pretty stuck with this for now.
+    #
+    # Patches to make this work more nicely with Ruby 1.9 using runtime
+    # version checking and all are welcome, but they can't actually help if
+    # the results are not totally hidden away in here.
+    #
+    # Incidentally, we though about vendoring evil-ruby and actually adjusting
+    # the internal C structure implementation details under the hood to make
+    # this stuff work, because it would have been cleaner.  Which gives you an
+    # idea how motivated we were to make this cleaner.  Sorry. --daniel 2011-03-31
+
+    internal_name = "#{@name} implementation, required on Ruby 1.8".to_sym
+    file = __FILE__ + "+eval"
+    line = __LINE__ + 1
+    wrapper = "def #{@name}(*args, &block)
+                 args << {} unless args.last.is_a? Hash
+                 args << block if block_given?
+                 self.__send__(#{internal_name.inspect}, *args)
+               end"
+
     if @string.is_a?(Class)
-      @string.define_method(@name, &block)
+      @string.class_eval do eval wrapper, nil, file, line end
+      @string.define_method(internal_name, &block)
     else
-      @string.meta_def(@name, &block)
+      @string.instance_eval do eval wrapper, nil, file, line end
+      @string.meta_def(internal_name, &block)
+    end
+  end
+
+  def add_option(option)
+    option.aliases.each do |name|
+      if conflict = get_option(name) then
+        raise ArgumentError, "Option #{option} conflicts with existing option #{conflict}"
+      elsif conflict = @string.get_option(name) then
+        raise ArgumentError, "Option #{option} conflicts with existing option #{conflict} on #{@string}"
+      end
     end
+
+    option.aliases.each do |name|
+      @options[name] = option
+    end
+
+    option
+  end
+
+  def option?(name)
+    @options.include? name.to_sym
+  end
+
+  def options
+    (@options.keys + @string.options).sort
+  end
+
+  def get_option(name)
+    @options[name.to_sym] || @string.get_option(name)
   end
 end
diff --git a/lib/puppet/string/action_builder.rb b/lib/puppet/string/action_builder.rb
index b3db51104..e76044470 100644
--- a/lib/puppet/string/action_builder.rb
+++ b/lib/puppet/string/action_builder.rb
@@ -5,10 +5,8 @@ class Puppet::String::ActionBuilder
   attr_reader :action
 
   def self.build(string, name, &block)
-    name = name.to_s
-    raise "Action '#{name}' must specify a block" unless block
-    builder = new(string, name, &block)
-    builder.action
+    raise "Action #{name.inspect} must specify a block" unless block
+    new(string, name, &block).action
   end
 
   def initialize(string, name, &block)
@@ -24,4 +22,9 @@ class Puppet::String::ActionBuilder
     raise "Invoke called on an ActionBuilder with no corresponding Action" unless @action
     @action.invoke = block
   end
+
+  def option(*declaration, &block)
+    option = Puppet::String::OptionBuilder.build(@action, *declaration, &block)
+    @action.add_option(option)
+  end
 end
diff --git a/lib/puppet/string/action_manager.rb b/lib/puppet/string/action_manager.rb
index c29dbf454..7d22a0c52 100644
--- a/lib/puppet/string/action_manager.rb
+++ b/lib/puppet/string/action_manager.rb
@@ -5,20 +5,15 @@ module Puppet::String::ActionManager
   # the code to do so.
   def action(name, &block)
     @actions ||= {}
-    name = name.to_s.downcase.to_sym
-
     raise "Action #{name} already defined for #{self}" if action?(name)
-
     action = Puppet::String::ActionBuilder.build(self, name, &block)
-
-    @actions[name] = action
+    @actions[action.name] = action
   end
 
   # This is the short-form of an action definition; it doesn't use the
   # builder, just creates the action directly from the block.
   def script(name, &block)
     @actions ||= {}
-    name = name.to_s.downcase.to_sym
     raise "Action #{name} already defined for #{self}" if action?(name)
     @actions[name] = Puppet::String::Action.new(self, name, :invoke => block)
   end
@@ -36,7 +31,16 @@ module Puppet::String::ActionManager
   end
 
   def get_action(name)
-    @actions[name].dup
+    @actions ||= {}
+    result = @actions[name.to_sym]
+    if result.nil?
+      if self.is_a?(Class) and superclass.respond_to?(:get_action)
+        result = superclass.get_action(name)
+      elsif self.class.respond_to?(:get_action)
+        result = self.class.get_action(name)
+      end
+    end
+    return result
   end
 
   def action?(name)
diff --git a/lib/puppet/string/catalog.rb b/lib/puppet/string/catalog.rb
index 0ddd83176..c6de47708 100644
--- a/lib/puppet/string/catalog.rb
+++ b/lib/puppet/string/catalog.rb
@@ -2,7 +2,7 @@ require 'puppet/string/indirector'
 
 Puppet::String::Indirector.define(:catalog, '0.0.1') do
   action(:apply) do
-    invoke do |catalog|
+    invoke do |catalog, options|
       report = Puppet::Transaction::Report.new("apply")
       report.configuration_version = catalog.version
 
@@ -23,7 +23,7 @@ Puppet::String::Indirector.define(:catalog, '0.0.1') do
   end
 
   action(:download) do
-    invoke do |certname,facts|
+    invoke do |certname, facts, options|
       Puppet::Resource::Catalog.terminus_class = :rest
       facts_to_upload = {:facts_format => :b64_zlib_yaml, :facts => CGI.escape(facts.render(:b64_zlib_yaml))}
       catalog = nil
diff --git a/lib/puppet/string/catalog/select.rb b/lib/puppet/string/catalog/select.rb
index 52c77d3ce..a8f4480cd 100644
--- a/lib/puppet/string/catalog/select.rb
+++ b/lib/puppet/string/catalog/select.rb
@@ -1,7 +1,7 @@
 # Select and show a list of resources of a given type.
 Puppet::String.define(:catalog, '0.0.1') do
   action :select do
-    invoke do |host,type|
+    invoke do |host, type, options|
       catalog = Puppet::Resource::Catalog.indirection.find(host)
 
       catalog.resources.reject { |res| res.type != type }.each { |res| puts res }
diff --git a/lib/puppet/string/certificate.rb b/lib/puppet/string/certificate.rb
index 7b2e5f397..53f731e81 100644
--- a/lib/puppet/string/certificate.rb
+++ b/lib/puppet/string/certificate.rb
@@ -4,7 +4,7 @@ require 'puppet/ssl/host'
 Puppet::String::Indirector.define(:certificate, '0.0.1') do
 
   action :generate do
-    invoke do |name|
+    invoke do |name, options|
       host = Puppet::SSL::Host.new(name)
       host.generate_certificate_request
       host.certificate_request.class.indirection.save(host.certificate_request)
@@ -12,7 +12,7 @@ Puppet::String::Indirector.define(:certificate, '0.0.1') do
   end
 
   action :list do
-    invoke do
+    invoke do |options|
       Puppet::SSL::Host.indirection.search("*", {
         :for => :certificate_request,
       }).map { |h| h.inspect }
@@ -20,7 +20,7 @@ Puppet::String::Indirector.define(:certificate, '0.0.1') do
   end
 
   action :sign do
-    invoke do |name|
+    invoke do |name, options|
       Puppet::SSL::Host.indirection.save(Puppet::SSL::Host.new(name))
     end
   end
diff --git a/lib/puppet/string/config.rb b/lib/puppet/string/config.rb
index ae1a408cf..49a1688fc 100644
--- a/lib/puppet/string/config.rb
+++ b/lib/puppet/string/config.rb
@@ -3,6 +3,7 @@ require 'puppet/string'
 Puppet::String.define(:config, '0.0.1') do
   action(:print) do
     invoke do |*args|
+      options = args.pop
       Puppet.settings[:configprint] = args.join(",")
       Puppet.settings.print_config_options
       nil
diff --git a/lib/puppet/string/configurer.rb b/lib/puppet/string/configurer.rb
index a6ea74b6a..2520d4188 100644
--- a/lib/puppet/string/configurer.rb
+++ b/lib/puppet/string/configurer.rb
@@ -2,7 +2,7 @@ require 'puppet/string'
 
 Puppet::String.define(:configurer, '0.0.1') do
   action(:synchronize) do
-    invoke do |certname|
+    invoke do |certname, options|
       facts = Puppet::String[:facts, '0.0.1'].find(certname)
       catalog = Puppet::String[:catalog, '0.0.1'].download(certname, facts)
       report = Puppet::String[:catalog, '0.0.1'].apply(catalog)
diff --git a/lib/puppet/string/facts.rb b/lib/puppet/string/facts.rb
index 73acb0df6..31298813b 100644
--- a/lib/puppet/string/facts.rb
+++ b/lib/puppet/string/facts.rb
@@ -6,7 +6,7 @@ Puppet::String::Indirector.define(:facts, '0.0.1') do
 
   # Upload our facts to the server
   action(:upload) do
-    invoke do |*args|
+    invoke do |options|
       Puppet::Node::Facts.indirection.terminus_class = :facter
       facts = Puppet::Node::Facts.indirection.find(Puppet[:certname])
       Puppet::Node::Facts.indirection.terminus_class = :rest
diff --git a/lib/puppet/string/indirector.rb b/lib/puppet/string/indirector.rb
index 15984e39e..bb081533f 100644
--- a/lib/puppet/string/indirector.rb
+++ b/lib/puppet/string/indirector.rb
@@ -2,6 +2,11 @@ require 'puppet'
 require 'puppet/string'
 
 class Puppet::String::Indirector < Puppet::String
+  option "--terminus TERMINUS" do
+    desc "REVISIT: You can select a terminus, which has some bigger effect
+that we should describe in this file somehow."
+  end
+
   def self.indirections
     Puppet::Indirector::Indirection.instances.collect { |t| t.to_s }.sort
   end
@@ -10,6 +15,21 @@ class Puppet::String::Indirector < Puppet::String
     Puppet::Indirector::Terminus.terminus_classes(indirection.to_sym).collect { |t| t.to_s }.sort
   end
 
+  def call_indirection_method(method, *args)
+    options = args.pop
+    options.has_key?(:terminus) and set_terminus(options[:terminus])
+
+    begin
+      result = indirection.__send__(method, *args)
+    rescue => detail
+      puts detail.backtrace if Puppet[:trace]
+      raise "Could not call '#{method}' on '#{indirection_name}': #{detail}"
+    end
+
+    indirection.reset_terminus_class
+    return result
+  end
+
   action :destroy do
     invoke { |*args| call_indirection_method(:destroy, *args) }
   end
@@ -29,11 +49,16 @@ class Puppet::String::Indirector < Puppet::String
   # Print the configuration for the current terminus class
   action :info do
     invoke do |*args|
+      options = args.pop
+      options.has_key?(:terminus) and set_terminus(options[:terminus])
+
       if t = indirection.terminus_class
         puts "Run mode '#{Puppet.run_mode.name}': #{t}"
       else
         $stderr.puts "No default terminus class for run mode '#{Puppet.run_mode.name}'"
       end
+
+      indirection.reset_terminus_class
     end
   end
 
@@ -53,7 +78,8 @@ class Puppet::String::Indirector < Puppet::String
   # One usually does.
   def indirection
     unless @indirection
-      Puppet.info("Could not find terminus for #{indirection_name}") unless @indirection = Puppet::Indirector::Indirection.instance(indirection_name)
+      @indirection = Puppet::Indirector::Indirection.instance(indirection_name)
+      @indirection or raise "Could not find terminus for #{indirection_name}"
     end
     @indirection
   end
@@ -62,18 +88,7 @@ class Puppet::String::Indirector < Puppet::String
     begin
       indirection.terminus_class = from
     rescue => detail
-      raise "Could not set '#{indirection.name}' terminus to '#{from}' (#{detail}); valid terminus types are #{terminus_classes(indirection.name).join(", ") }"
+      raise "Could not set '#{indirection.name}' terminus to '#{from}' (#{detail}); valid terminus types are #{self.class.terminus_classes(indirection.name).join(", ") }"
     end
   end
-
-  def call_indirection_method(method, *args)
-    begin
-      result = indirection.send(method, *args)
-    rescue => detail
-      puts detail.backtrace if Puppet[:trace]
-      raise "Could not call '#{method}' on '#{indirection_name}': #{detail}"
-    end
-
-    result
-  end
 end
diff --git a/lib/puppet/string/option.rb b/lib/puppet/string/option.rb
new file mode 100644
index 000000000..f499e4b95
--- /dev/null
+++ b/lib/puppet/string/option.rb
@@ -0,0 +1,80 @@
+class Puppet::String::Option
+  attr_reader   :parent
+  attr_reader   :name
+  attr_reader   :aliases
+  attr_reader   :optparse
+  attr_accessor :desc
+
+  def takes_argument?
+    !!@argument
+  end
+  def optional_argument?
+    !!@optional_argument
+  end
+
+  def initialize(parent, *declaration, &block)
+    @parent   = parent
+    @optparse = []
+
+    # Collect and sort the arguments in the declaration.
+    dups = {}
+    declaration.each do |item|
+      if item.is_a? String and item.to_s =~ /^-/ then
+        unless item =~ /^-[a-z]\b/ or item =~ /^--[^-]/ then
+          raise ArgumentError, "#{item.inspect}: long options need two dashes (--)"
+        end
+        @optparse << item
+
+        # Duplicate checking...
+        name = optparse_to_name(item)
+        if dup = dups[name] then
+          raise ArgumentError, "#{item.inspect}: duplicates existing alias #{dup.inspect} in #{@parent}"
+        else
+          dups[name] = item
+        end
+      else
+        raise ArgumentError, "#{item.inspect} is not valid for an option argument"
+      end
+    end
+
+    if @optparse.empty? then
+      raise ArgumentError, "No option declarations found while building"
+    end
+
+    # Now, infer the name from the options; we prefer the first long option as
+    # the name, rather than just the first option.
+    @name = optparse_to_name(@optparse.find do |a| a =~ /^--/ end || @optparse.first)
+    @aliases = @optparse.map { |o| optparse_to_name(o) }
+
+    # Do we take an argument?  If so, are we consistent about it, because
+    # incoherence here makes our life super-difficult, and we can more easily
+    # relax this rule later if we find a valid use case for it. --daniel 2011-03-30
+    @argument = @optparse.any? { |o| o =~ /[ =]/ }
+    if @argument and not @optparse.all? { |o| o =~ /[ =]/ } then
+      raise ArgumentError, "Option #{@name} is inconsistent about taking an argument"
+    end
+
+    # Is our argument optional?  The rules about consistency apply here, also,
+    # just like they do to taking arguments at all. --daniel 2011-03-30
+    @optional_argument = @optparse.any? { |o| o.include? "[" }
+    if @optional_argument and not @optparse.all? { |o| o.include? "[" } then
+      raise ArgumentError, "Option #{@name} is inconsistent about the argument being optional"
+    end
+  end
+
+  # to_s and optparse_to_name are roughly mirrored, because they are used to
+  # transform strings to name symbols, and vice-versa.  This isn't a full
+  # bidirectional transformation though.
+  def to_s
+    @name.to_s.tr('_', '-')
+  end
+
+  def optparse_to_name(declaration)
+    unless found = declaration.match(/^-+([^= ]+)/) or found.length != 1 then
+      raise ArgumentError, "Can't find a name in the declaration #{declaration.inspect}"
+    end
+    name = found.captures.first.sub('[no-]', '').tr('-', '_')
+    raise "#{name.inspect} is an invalid option name" unless name.to_s =~ /^[a-z]\w*$/
+    name.to_sym
+  end
+end
diff --git a/lib/puppet/string/option_builder.rb b/lib/puppet/string/option_builder.rb
new file mode 100644
index 000000000..da0d213fb
--- /dev/null
+++ b/lib/puppet/string/option_builder.rb
@@ -0,0 +1,25 @@
+require 'puppet/string/option'
+
+class Puppet::String::OptionBuilder
+  attr_reader :option
+
+  def self.build(string, *declaration, &block)
+    new(string, *declaration, &block).option
+  end
+
+  private
+  def initialize(string, *declaration, &block)
+    @string = string
+    @option = Puppet::String::Option.new(string, *declaration)
+    block and instance_eval(&block)
+    @option
+  end
+
+  # Metaprogram the simple DSL from the option class.
+  Puppet::String::Option.instance_methods.grep(/=$/).each do |setter|
+    next if setter =~ /^=/      # special case, darn it...
+
+    dsl = setter.sub(/=$/, '')
+    define_method(dsl) do |value| @option.send(setter, value) end
+  end
+end
diff --git a/lib/puppet/string/option_manager.rb b/lib/puppet/string/option_manager.rb
new file mode 100644
index 000000000..f952ad4f0
--- /dev/null
+++ b/lib/puppet/string/option_manager.rb
@@ -0,0 +1,56 @@
+require 'puppet/string/option_builder'
+
+module Puppet::String::OptionManager
+  # Declare that this app can take a specific option, and provide
+  # the code to do so.
+  def option(*declaration, &block)
+    add_option Puppet::String::OptionBuilder.build(self, *declaration, &block)
+  end
+
+  def add_option(option)
+    option.aliases.each do |name|
+      if conflict = get_option(name) then
+        raise ArgumentError, "Option #{option} conflicts with existing option #{conflict}"
+      end
+
+      actions.each do |action|
+        action = get_action(action)
+        if conflict = action.get_option(name) then
+          raise ArgumentError, "Option #{option} conflicts with existing option #{conflict} on #{action}"
+        end
+      end
+    end
+
+    option.aliases.each { |name| @options[name] = option }
+    option
+  end
+
+  def options
+    @options ||= {}
+    result = @options.keys
+
+    if self.is_a?(Class) and superclass.respond_to?(:options)
+      result += superclass.options
+    elsif self.class.respond_to?(:options)
+      result += self.class.options
+    end
+    result.sort
+  end
+
+  def get_option(name)
+    @options ||= {}
+    result = @options[name.to_sym]
+    unless result then
+      if self.is_a?(Class) and superclass.respond_to?(:get_option)
+        result = superclass.get_option(name)
+      elsif self.class.respond_to?(:get_option)
+        result = self.class.get_option(name)
+      end
+    end
+    return result
+  end
+
+  def option?(name)
+    options.include? name.to_sym
+  end
+end
diff --git a/lib/puppet/string/report.rb b/lib/puppet/string/report.rb
index 55a008533..5b617e49e 100644
--- a/lib/puppet/string/report.rb
+++ b/lib/puppet/string/report.rb
@@ -2,7 +2,7 @@ require 'puppet/string/indirector'
 
 Puppet::String::Indirector.define(:report, '0.0.1') do
   action(:submit) do
-    invoke do |report|
+    invoke do |report, options|
       begin
         Puppet::Transaction::Report.terminus_class = :rest
         report.save