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
@@ -1,23 +1,23 @@
 require 'puppet/application'
 require 'puppet/string'
 
 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]
       Puppet::Util::Log.level = options[:debug] ? :debug : :info
     end
 
     Puppet::Util::Log.newdestination(:console)
   end
 
   def run_command
     report = Puppet::String[:configurer, '0.0.1'].synchronize(Puppet[:certname])
     Puppet::String[:report, '0.0.1'].submit(report)
   end
 end
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
@@ -1,95 +1,95 @@
 require 'puppet/application'
 require 'puppet/string'
 
 class Puppet::Application::String < Puppet::Application
 
   should_parse_config
   run_mode :agent
 
   option("--debug", "-d") do |arg|
     Puppet::Util::Log.level = :debug
   end
 
   option("--verbose", "-v") do
     Puppet::Util::Log.level = :info
   end
 
   def list(*arguments)
     if arguments.empty?
       arguments = %w{terminuses actions}
     end
     strings.each do |name|
       str = "#{name}:\n"
       if arguments.include?("terminuses")
         begin
           terms = terminus_classes(name.to_sym)
           str << "\tTerminuses: #{terms.join(", ")}\n"
         rescue => detail
           puts detail.backtrace if Puppet[:trace]
           $stderr.puts "Could not load terminuses for #{name}: #{detail}"
         end
       end
 
       if arguments.include?("actions")
         begin
           actions = actions(name.to_sym)
           str << "\tActions: #{actions.join(", ")}\n"
         rescue => detail
           puts detail.backtrace if Puppet[:trace]
           $stderr.puts "Could not load actions for #{name}: #{detail}"
         end
       end
 
       print str
     end
   end
 
   attr_accessor :verb, :name, :arguments
 
   def main
     # Call the method associated with the provided action (e.g., 'find').
     send(verb, *arguments)
   end
 
   def setup
     Puppet::Util::Log.newdestination :console
 
     load_applications # Call this to load all of the apps
 
     @verb, @arguments = command_line.args
     @arguments ||= []
 
     validate
   end
 
   def validate
     unless verb
       raise "You must specify 'find', 'search', 'save', or 'destroy' as a verb; 'save' probably does not work right now"
     end
 
     unless respond_to?(verb)
       raise "Command '#{verb}' not found for 'string'"
     end
   end
 
   def strings
     Puppet::String.strings
   end
 
   def terminus_classes(indirection)
     Puppet::Indirector::Terminus.terminus_classes(indirection).collect { |t| t.to_s }.sort
   end
 
   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
     command_line.available_subcommands.each do |app|
       command_line.require_application app
     end
   end
 end
 
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
@@ -1,97 +1,128 @@
 require 'puppet/application'
 require 'puppet/string'
 
 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
 
   option("--verbose", "-v") do
     Puppet::Util::Log.level = :info
   end
 
   option("--format FORMAT") do |arg|
     @format = arg.to_sym
   end
 
   option("--mode RUNMODE", "-r") do |arg|
     raise "Invalid run mode #{arg}; supported modes are user, agent, master" unless %w{user agent master}.include?(arg)
     self.class.run_mode(arg.to_sym)
     set_run_mode self.class.run_mode
   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
   # immediately but you need to indicate a failure.
   def exit_code
     @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
     if render_method == "to_pson"
       jj result
       exit(0)
     else
       result.send(render_method)
     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
@@ -1,100 +1,104 @@
 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
     # This is just so we can search for actions.  We only use its
     # list of directories to search.
     # Can't we utilize an external autoloader, or simply use the $LOAD_PATH? -pvb
     def autoloader
       @autoloader ||= Puppet::Util::Autoload.new(:application, "puppet/string")
     end
 
     def strings
       Puppet::String::StringCollection.strings
     end
 
     def string?(name, version)
       Puppet::String::StringCollection.string?(name, version)
     end
 
     def register(instance)
       Puppet::String::StringCollection.register(instance)
     end
 
     def define(name, version, &block)
       if string?(name, version)
         string = Puppet::String::StringCollection[name, version]
       else
         string = self.new(name, version)
         Puppet::String::StringCollection.register(string)
         string.load_actions
       end
 
       string.instance_eval(&block) if block_given?
 
       return string
     end
 
     alias :[] :define
   end
 
   attr_accessor :default_format
 
   def set_default_format(format)
     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)
     @version = version
     @default_format = :pson
 
     instance_eval(&block) if block_given?
   end
 
   # Try to find actions defined in other files.
   def load_actions
     path = "puppet/string/#{name}"
 
     loaded = []
     [path, "#{name}@#{version}/#{path}"].each do |path|
       Puppet::String.autoloader.search_directories.each do |dir|
         fdir = ::File.join(dir, path)
         next unless FileTest.directory?(fdir)
 
         Dir.chdir(fdir) do
           Dir.glob("*.rb").each do |file|
             aname = file.sub(/\.rb/, '')
             if loaded.include?(aname)
               Puppet.debug "Not loading duplicate action '#{aname}' for '#{name}' from '#{fdir}/#{file}'"
               next
             end
             loaded << aname
             Puppet.debug "Loading action '#{aname}' for '#{name}' from '#{fdir}/#{file}'"
             require "#{Dir.pwd}/#{aname}"
           end
         end
       end
     end
   end
 
   def to_s
     "Puppet::String[#{name.inspect}, #{version.inspect}]"
   end
 end
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
@@ -1,27 +1,30 @@
 require 'puppet/string'
 require 'puppet/string/action'
 
 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)
     @string = string
     @action = Puppet::String::Action.new(string, name)
     instance_eval(&block)
   end
 
   # Ideally the method we're defining here would be added to the action, and a
   # method on the string would defer to it, but we can't get scope correct,
   # so we stick with this. --daniel 2011-03-24
   def invoke(&block)
     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
@@ -1,45 +1,49 @@
 require 'puppet/string/action_builder'
 
 module Puppet::String::ActionManager
   # Declare that this app can take a specific action, and provide
   # 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
 
   def actions
     @actions ||= {}
     result = @actions.keys
 
     if self.is_a?(Class) and superclass.respond_to?(:actions)
       result += superclass.actions
     elsif self.class.respond_to?(:actions)
       result += self.class.actions
     end
     result.sort
   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)
     actions.include?(name.to_sym)
   end
 end
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
@@ -1,40 +1,40 @@
 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
 
       Puppet::Util::Log.newdestination(report)
 
       begin
         benchmark(:notice, "Finished catalog run") do
           catalog.apply(:report => report)
         end
       rescue => detail
         puts detail.backtrace if Puppet[:trace]
         Puppet.err "Failed to apply catalog: #{detail}"
       end
 
       report.finalize_report
       report
     end
   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
       retrieval_duration = thinmark do
         catalog = Puppet::String[:catalog, '0.0.1'].find(certname, facts_to_upload)
       end
       catalog = catalog.to_ral
       catalog.finalize
       catalog.retrieval_duration = retrieval_duration
       catalog.write_class_file
       catalog
     end
   end
 end
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,10 +1,10 @@
 # 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 }
     end
   end
 end
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
@@ -1,28 +1,28 @@
 require 'puppet/string/indirector'
 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)
     end
   end
 
   action :list do
-    invoke do
+    invoke do |options|
       Puppet::SSL::Host.indirection.search("*", {
         :for => :certificate_request,
       }).map { |h| h.inspect }
     end
   end
 
   action :sign do
-    invoke do |name|
+    invoke do |name, options|
       Puppet::SSL::Host.indirection.save(Puppet::SSL::Host.new(name))
     end
   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
@@ -1,11 +1,12 @@
 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
     end
   end
 end
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
@@ -1,12 +1,12 @@
 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)
       report
     end
   end
 end
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
@@ -1,18 +1,18 @@
 require 'puppet/string/indirector'
 require 'puppet/node/facts'
 
 Puppet::String::Indirector.define(:facts, '0.0.1') do
   set_default_format :yaml
 
   # 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
       Puppet::Node::Facts.indirection.save(facts)
       Puppet.notice "Uploaded facts for '#{Puppet[:certname]}'"
       nil
     end
   end
 end
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
@@ -1,79 +1,94 @@
 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
 
   def self.terminus_classes(indirection)
     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
 
   action :find do
     invoke { |*args| call_indirection_method(:find, *args) }
   end
 
   action :save do
     invoke { |*args| call_indirection_method(:save, *args) }
   end
 
   action :search do
     invoke { |*args| call_indirection_method(:search, *args) }
   end
 
   # 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
 
   attr_accessor :from
 
   def indirection_name
     @indirection_name || name.to_sym
   end
 
   # Here's your opportunity to override the indirection name.  By default
   # it will be the same name as the string.
   def set_indirection_name(name)
     @indirection_name = name
   end
 
   # Return an indirection associated with an string, if one exists
   # 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
 
   def set_terminus(from)
     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
@@ -1,15 +1,15 @@
 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
       rescue => detail
         puts detail.backtrace if Puppet[:trace]
         Puppet.err "Could not send report: #{detail}"
       end
     end
   end
 end
diff --git a/spec/shared_behaviours/things_that_declare_options.rb b/spec/shared_behaviours/things_that_declare_options.rb
new file mode 100644
index 000000000..1b41c2279
--- /dev/null
+++ b/spec/shared_behaviours/things_that_declare_options.rb
@@ -0,0 +1,134 @@
+# -*- coding: utf-8 -*-
+shared_examples_for "things that declare options" do
+  it "should support options without arguments" do
+    subject = add_options_to { option "--bar" }
+    subject.should be_option :bar
+  end
+
+  it "should support options with an empty block" do
+    subject = add_options_to do
+      option "--foo" do
+        # this section deliberately left blank
+      end
+    end
+    subject.should be
+    subject.should be_option :foo
+  end
+
+  it "should support option documentation" do
+    text = "Sturm und Drang (German pronunciation: [ˈʃtʊʁm ʊnt ˈdʁaŋ]) …"
+
+    subject = add_options_to do
+      option "--foo" do
+        desc text
+      end
+    end
+
+    subject.get_option(:foo).desc.should == text
+  end
+
+  it "should list all the options" do
+    subject = add_options_to do
+      option "--foo"
+      option "--bar"
+    end
+    subject.options.should =~ [:foo, :bar]
+  end
+
+  it "should detect conflicts in long options" do
+    expect {
+      add_options_to do
+        option "--foo"
+        option "--foo"
+      end
+    }.should raise_error ArgumentError, /Option foo conflicts with existing option foo/i
+  end
+
+  it "should detect conflicts in short options" do
+    expect {
+      add_options_to do
+        option "-f"
+        option "-f"
+      end
+    }.should raise_error ArgumentError, /Option f conflicts with existing option f/
+  end
+
+  ["-f", "--foo"].each do |option|
+    ["", " FOO", "=FOO", " [FOO]", "=[FOO]"].each do |argument|
+      input = option + argument
+      it "should detect conflicts within a single option like #{input.inspect}" do
+        expect {
+          add_options_to do
+            option input, input
+          end
+        }.should raise_error ArgumentError, /duplicates existing alias/
+      end
+    end
+  end
+
+
+  # Verify the range of interesting conflicts to check for ordering causing
+  # the behaviour to change, or anything exciting like that.
+  [ %w{--foo}, %w{-f}, %w{-f --foo}, %w{--baz -f},
+    %w{-f --baz}, %w{-b --foo}, %w{--foo -b}
+  ].each do |conflict|
+    base = %w{--foo -f}
+    it "should detect conflicts between #{base.inspect} and #{conflict.inspect}" do
+      expect {
+        add_options_to do
+          option *base
+          option *conflict
+        end
+      }.should raise_error ArgumentError, /conflicts with existing option/
+    end
+  end
+
+  it "should fail if we are not consistent about taking an argument" do
+    expect { add_options_to do option "--foo=bar", "--bar" end }.
+      should raise_error ArgumentError, /inconsistent about taking an argument/
+  end
+
+  it "should accept optional arguments" do
+    subject = add_options_to do option "--foo=[baz]", "--bar=[baz]" end
+    [:foo, :bar].each do |name|
+      subject.should be_option name
+    end
+  end
+
+  describe "#takes_argument?" do
+    it "should detect an argument being absent" do
+      subject = add_options_to do option "--foo" end
+      subject.get_option(:foo).should_not be_takes_argument
+    end
+    ["=FOO", " FOO", "=[FOO]", " [FOO]"].each do |input|
+      it "should detect an argument given #{input.inspect}" do
+        subject = add_options_to do option "--foo#{input}" end
+        subject.get_option(:foo).should be_takes_argument
+      end
+    end
+  end
+
+  describe "#optional_argument?" do
+    it "should be false if no argument is present" do
+      option = add_options_to do option "--foo" end.get_option(:foo)
+      option.should_not be_takes_argument
+      option.should_not be_optional_argument
+    end
+
+    ["=FOO", " FOO"].each do |input|
+      it "should be false if the argument is mandatory (like #{input.inspect})" do
+        option = add_options_to do option "--foo#{input}" end.get_option(:foo)
+      option.should be_takes_argument
+      option.should_not be_optional_argument
+      end
+    end
+
+    ["=[FOO]", " [FOO]"].each do |input|
+      it "should be true if the argument is optional (like #{input.inspect})" do
+        option = add_options_to do option "--foo#{input}" end.get_option(:foo)
+      option.should be_takes_argument
+      option.should be_optional_argument
+      end
+    end
+  end
+end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 4e54d7235..4073cb60b 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -1,38 +1,42 @@
 require 'pathname'
 dir = Pathname.new(__FILE__).parent
 $LOAD_PATH.unshift(dir, dir + 'lib', dir + '../lib')
 
 require 'mocha'
 require 'puppet'
 require 'puppet/string'
 require 'rspec'
 
+Pathname.glob("#{dir}/shared_behaviours/**/*.rb") do |behaviour|
+  require behaviour.relative_path_from(dir)
+end
+
 RSpec.configure do |config|
     config.mock_with :mocha
 
     config.before :each do
       # Set the confdir and vardir to gibberish so that tests
       # have to be correctly mocked.
       Puppet[:confdir] = "/dev/null"
       Puppet[:vardir] = "/dev/null"
 
       # Avoid opening ports to the outside world
       Puppet.settings[:bindaddress] = "127.0.0.1"
 
       @logs = []
       Puppet::Util::Log.newdestination(@logs)
     end
 
     config.after :each do
       Puppet.settings.clear
 
       @logs.clear
       Puppet::Util::Log.close_all
     end
 end
 
 # We need this because the RAL uses 'should' as a method.  This
 # allows us the same behaviour but with a different method name.
 class Object
     alias :must :should
 end
diff --git a/spec/unit/application/certificate_spec.rb b/spec/unit/application/certificate_spec.rb
old mode 100644
new mode 100755
index 0688666de..6666f54f7
--- a/spec/unit/application/certificate_spec.rb
+++ b/spec/unit/application/certificate_spec.rb
@@ -1,21 +1,16 @@
 #!/usr/bin/env ruby
 
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/application/certificate'
 
 describe Puppet::Application::Certificate do
   it "should be a subclass of Puppet::Application::IndirectionBase" do
     Puppet::Application::Certificate.superclass.should equal(
       Puppet::Application::IndirectionBase
     )
   end
 
   it "should have a 'ca' option" do
     Puppet::Application::Certificate.new.should respond_to(:handle_ca_location)
   end
-
-  it "should set the CA location using the 'ca' option" do
-    Puppet::Application::Certificate.new.handle_ca_location("local")
-    Puppet::SSL::Host.indirection.terminus_class.should == :file
-  end
 end
diff --git a/spec/unit/application/configurer_spec.rb b/spec/unit/application/configurer_spec.rb
old mode 100644
new mode 100755
diff --git a/spec/unit/application/indirection_base_spec.rb b/spec/unit/application/indirection_base_spec.rb
index ecc49d9a9..66b3009fb 100755
--- a/spec/unit/application/indirection_base_spec.rb
+++ b/spec/unit/application/indirection_base_spec.rb
@@ -1,12 +1,39 @@
 #!/usr/bin/env ruby
 
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/application/indirection_base'
+require 'puppet/string/indirector'
+
+########################################################################
+# Stub for testing; the names are critical, sadly. --daniel 2011-03-30
+class Puppet::Application::TestIndirection < Puppet::Application::IndirectionBase
+end
+
+string = Puppet::String::Indirector.define(:testindirection, '0.0.1') do
+end
+# REVISIT: This horror is required because we don't allow anything to be
+# :current except for if it lives on, and is loaded from, disk. --daniel 2011-03-29
+string.version = :current
+Puppet::String.register(string)
+########################################################################
+
 
 describe Puppet::Application::IndirectionBase do
-  it "should support a 'from' terminus"
+  subject { Puppet::Application::TestIndirection.new }
+
+  it "should accept a terminus command line option" do
+    # It would be nice not to have to stub this, but whatever... writing an
+    # entire indirection stack would cause us more grief. --daniel 2011-03-31
+    terminus = mock("test indirection terminus")
+    Puppet::Indirector::Indirection.expects(:instance).
+      with(:testindirection).twice.returns()
+
+    subject.command_line.
+      instance_variable_set('@args', %w{--terminus foo save})
+
+    # Not a very nice thing. :(
+    $stderr.stubs(:puts)
 
-  describe "setup" do
-    it "should fail if its string does not support an indirection"
+    expect { subject.run }.should raise_error SystemExit
   end
 end
diff --git a/spec/unit/application/string_base_spec.rb b/spec/unit/application/string_base_spec.rb
index 86f9c09aa..71e67283d 100755
--- a/spec/unit/application/string_base_spec.rb
+++ b/spec/unit/application/string_base_spec.rb
@@ -1,74 +1,150 @@
 #!/usr/bin/env ruby
 
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/application/string_base'
 require 'tmpdir'
 
 class Puppet::Application::StringBase::Basetest < Puppet::Application::StringBase
 end
 
 describe Puppet::Application::StringBase do
   before :all do
     @dir = Dir.mktmpdir
     $LOAD_PATH.push(@dir)
     FileUtils.mkdir_p(File.join @dir, 'puppet', 'string')
     File.open(File.join(@dir, 'puppet', 'string', 'basetest.rb'), 'w') do |f|
       f.puts "Puppet::String.define(:basetest, '0.0.1')"
     end
+
+    Puppet::String.define(:basetest, '0.0.1') do
+      option("--[no-]boolean")
+      option("--mandatory MANDATORY")
+      option("--optional [OPTIONAL]")
+
+      action :foo do
+        option("--action")
+        invoke { |*args| args.length }
+      end
+    end
   end
 
   after :all do
     FileUtils.remove_entry_secure @dir
     $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'
     Puppet::Util::Log.stubs(:newdestination)
+    app
   end
 
-  describe "when calling main" do
-    before do
-      @app.verb = :find
-      @app.arguments = ["myname", "myarg"]
-      @app.string.stubs(:find)
+  describe "#preinit" do
+    before :each do
+      app.command_line.stubs(:args).returns %w{}
     end
 
-    it "should send the specified verb and name to the string" do
-      @app.string.expects(:find).with("myname", "myarg")
+    describe "parsing the command line" do
+      context "with just an action" do
+        before :all do
+          app.command_line.stubs(:args).returns %w{foo}
+          app.preinit
+        end
 
-      @app.main
-    end
+        it "should set the string based on the type" do
+          app.string.name.should == :basetest
+        end
 
-    it "should use its render method to render any result"
+        it "should set the format based on the string default" do
+          app.format.should == :pson
+        end
 
-    it "should exit with the current exit code"
-  end
+        it "should find the action" do
+          app.action.should be
+          app.action.name.should == :foo
+        end
+      end
 
-  describe "during setup" do
-    before do
-      @app.command_line.stubs(:args).returns(["find", "myname", "myarg"])
-      @app.stubs(:validate)
+      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{--action=bar foo}
+        expect { app.preinit }.
+          should raise_error ArgumentError, /Unknown option "--action"/
+      end
+
+      it "should fail if an action option is before the action" do
+        app.command_line.stubs(:args).returns %w{--action foo}
+        expect { app.preinit }.
+          should raise_error ArgumentError, /Unknown option "--action"/
+      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
+
+      it "should accept --bar as an argument to a mandatory option after action" do
+        app.command_line.stubs(:args).returns %w{foo --mandatory --bar}
+        app.preinit and app.parse_options
+        app.action.name.should == :foo
+        app.options.should == { :mandatory => "--bar" }
+      end
+
+      it "should accept --bar as an argument to a mandatory option before action" do
+        app.command_line.stubs(:args).returns %w{--mandatory --bar foo}
+        app.preinit and app.parse_options
+        app.action.name.should == :foo
+        app.options.should == { :mandatory => "--bar" }
+      end
+
+      it "should not skip when --foo=bar is given" do
+        app.command_line.stubs(:args).returns %w{--mandatory=bar --bar foo}
+        expect { app.preinit }.
+          should raise_error ArgumentError, /Unknown option "--bar"/
+      end
     end
+  end
 
-    it "should set the verb from the command line arguments" do
-      @app.setup
-      @app.verb.should == "find"
+  describe "#setup" do
+    it "should remove the action name from the arguments" do
+      app.command_line.stubs(:args).returns %w{--mandatory --bar foo}
+      app.preinit and app.parse_options and app.setup
+      app.arguments.should == [{ :mandatory => "--bar" }]
     end
+  end
 
-    it "should make sure arguments are an array" do
-      @app.command_line.stubs(:args).returns(["find", "myname", "myarg"])
-      @app.setup
-      @app.arguments.should == ["myname", "myarg"]
+  describe "#main" do
+    before do
+      app.string    = Puppet::String[:basetest, '0.0.1']
+      app.action    = app.string.get_action(:foo)
+      app.format    = :pson
+      app.arguments = ["myname", "myarg"]
     end
 
-    it "should set the options on the string" do
-      @app.options[:foo] = "bar"
-      @app.setup
+    it "should send the specified verb and name to the string" do
+      app.string.expects(:foo).with(*app.arguments)
+      app.main
+    end
 
-      @app.string.options.should == @app.options
+    it "should use its render method to render any result" do
+      app.expects(:render).with(app.arguments.length + 1)
+      app.main
     end
   end
 end
diff --git a/spec/unit/string/action_builder_spec.rb b/spec/unit/string/action_builder_spec.rb
index c3395cf6a..fde010d51 100755
--- a/spec/unit/string/action_builder_spec.rb
+++ b/spec/unit/string/action_builder_spec.rb
@@ -1,30 +1,59 @@
 #!/usr/bin/env ruby
 
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/string/action_builder'
 
 describe Puppet::String::ActionBuilder do
   describe "::build" do
     it "should build an action" do
-      action = Puppet::String::ActionBuilder.build(nil,:foo) do
+      action = Puppet::String::ActionBuilder.build(nil, :foo) do
       end
       action.should be_a(Puppet::String::Action)
-      action.name.should == "foo"
+      action.name.should == :foo
     end
 
     it "should define a method on the string which invokes the action" do
       string = Puppet::String.new(:action_builder_test_string, '0.0.1')
       action = Puppet::String::ActionBuilder.build(string, :foo) do
         invoke do
           "invoked the method"
         end
       end
 
       string.foo.should == "invoked the method"
     end
 
     it "should require a block" do
-      lambda { Puppet::String::ActionBuilder.build(nil,:foo) }.should raise_error("Action 'foo' must specify a block")
+      lambda { Puppet::String::ActionBuilder.build(nil, :foo) }.
+        should raise_error("Action :foo must specify a block")
+    end
+
+    describe "when handling options" do
+      let :string do Puppet::String.new(:option_handling, '0.0.1') end
+
+      it "should have a #option DSL function" do
+        method = nil
+        Puppet::String::ActionBuilder.build(string, :foo) do
+          method = self.method(:option)
+        end
+        method.should be
+      end
+
+      it "should define an option without a block" do
+        action = Puppet::String::ActionBuilder.build(string, :foo) do
+          option "--bar"
+        end
+        action.should be_option :bar
+      end
+
+      it "should accept an empty block" do
+        action = Puppet::String::ActionBuilder.build(string, :foo) do
+          option "--bar" do
+            # This space left deliberately blank.
+          end
+        end
+        action.should be_option :bar
+      end
     end
   end
 end
diff --git a/spec/unit/string/action_manager_spec.rb b/spec/unit/string/action_manager_spec.rb
index 3921f02c0..5ca55b387 100755
--- a/spec/unit/string/action_manager_spec.rb
+++ b/spec/unit/string/action_manager_spec.rb
@@ -1,216 +1,233 @@
 #!/usr/bin/env ruby
 
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 
 # This is entirely an internal class for String, so we have to load it instead of our class.
 require 'puppet/string'
 
 class ActionManagerTester
   include Puppet::String::ActionManager
 end
 
 describe Puppet::String::ActionManager do
   subject { ActionManagerTester.new }
 
   describe "when included in a class" do
     it "should be able to define an action" do
       subject.action(:foo) do
         invoke { "something "}
       end
     end
 
     it "should be able to define a 'script' style action" do
       subject.script :bar do
         "a bar is where beer is found"
       end
     end
 
     it "should be able to list defined actions" do
       subject.action(:foo) do
         invoke { "something" }
       end
       subject.action(:bar) do
         invoke { "something" }
       end
 
       subject.actions.should =~ [:foo, :bar]
     end
 
     it "should list 'script' actions" do
       subject.script :foo do "foo" end
       subject.actions.should =~ [:foo]
     end
 
     it "should list both script and normal actions" do
       subject.action :foo do
         invoke do "foo" end
       end
       subject.script :bar do "a bar is where beer is found" end
 
       subject.actions.should =~ [:foo, :bar]
     end
 
     it "should be able to indicate when an action is defined" do
       subject.action(:foo) do
         invoke { "something" }
       end
 
       subject.should be_action(:foo)
     end
 
     it "should indicate an action is defined for script actions" do
       subject.script :foo do "foo" end
       subject.should be_action :foo
     end
 
     it "should correctly treat action names specified as strings" do
       subject.action(:foo) do
         invoke { "something" }
       end
 
       subject.should be_action("foo")
     end
   end
 
   describe "when used to extend a class" do
     subject { Class.new.extend(Puppet::String::ActionManager) }
 
     it "should be able to define an action" do
       subject.action(:foo) do
         invoke { "something "}
       end
     end
 
     it "should be able to list defined actions" do
       subject.action(:foo) do
         invoke { "something" }
       end
       subject.action(:bar) do
         invoke { "something" }
       end
 
       subject.actions.should include(:bar)
       subject.actions.should include(:foo)
     end
 
     it "should be able to indicate when an action is defined" do
       subject.action(:foo) { "something" }
       subject.should be_action(:foo)
     end
   end
 
   describe "when used both at the class and instance level" do
     before do
       @klass = Class.new do
         include Puppet::String::ActionManager
         extend Puppet::String::ActionManager
       end
       @instance = @klass.new
     end
 
     it "should be able to define an action at the class level" do
       @klass.action(:foo) do
         invoke { "something "}
       end
     end
 
     it "should create an instance method when an action is defined at the class level" do
       @klass.action(:foo) do
         invoke { "something" }
       end
       @instance.foo.should == "something"
     end
 
     it "should be able to define an action at the instance level" do
       @instance.action(:foo) do
         invoke { "something "}
       end
     end
 
     it "should create an instance method when an action is defined at the instance level" do
       @instance.action(:foo) do
         invoke { "something" }
       end
       @instance.foo.should == "something"
     end
 
     it "should be able to list actions defined at the class level" do
       @klass.action(:foo) do
         invoke { "something" }
       end
       @klass.action(:bar) do
         invoke { "something" }
       end
 
       @klass.actions.should include(:bar)
       @klass.actions.should include(:foo)
     end
 
     it "should be able to list actions defined at the instance level" do
       @instance.action(:foo) do
         invoke { "something" }
       end
       @instance.action(:bar) do
         invoke { "something" }
       end
 
       @instance.actions.should include(:bar)
       @instance.actions.should include(:foo)
     end
 
     it "should be able to list actions defined at both instance and class level" do
       @klass.action(:foo) do
         invoke { "something" }
       end
       @instance.action(:bar) do
         invoke { "something" }
       end
 
       @instance.actions.should include(:bar)
       @instance.actions.should include(:foo)
     end
 
     it "should be able to indicate when an action is defined at the class level" do
       @klass.action(:foo) do
         invoke { "something" }
       end
       @instance.should be_action(:foo)
     end
 
     it "should be able to indicate when an action is defined at the instance level" do
       @klass.action(:foo) do
         invoke { "something" }
       end
       @instance.should be_action(:foo)
     end
 
     it "should list actions defined in superclasses" do
       @subclass = Class.new(@klass)
       @instance = @subclass.new
 
       @klass.action(:parent) do
         invoke { "a" }
       end
       @subclass.action(:sub) do
         invoke { "a" }
       end
       @instance.action(:instance) do
         invoke { "a" }
       end
 
       @instance.should be_action(:parent)
       @instance.should be_action(:sub)
       @instance.should be_action(:instance)
     end
 
     it "should create an instance method when an action is defined in a superclass" do
       @subclass = Class.new(@klass)
       @instance = @subclass.new
 
       @klass.action(:foo) do
         invoke { "something" }
       end
       @instance.foo.should == "something"
     end
   end
+
+  describe "#get_action" do
+    let :parent_class do
+      parent_class = Class.new(Puppet::String)
+      parent_class.action(:foo) {}
+      parent_class
+    end
+
+    it "should check that we can find inherited actions when we are a class" do
+      Class.new(parent_class).get_action(:foo).name.should == :foo
+    end
+
+    it "should check that we can find inherited actions when we are an instance" do
+      instance = parent_class.new(:foo, '0.0.0')
+      instance.get_action(:foo).name.should == :foo
+    end
+  end
 end
diff --git a/spec/unit/string/action_spec.rb b/spec/unit/string/action_spec.rb
index 4026c9a58..e5fefdbdc 100755
--- a/spec/unit/string/action_spec.rb
+++ b/spec/unit/string/action_spec.rb
@@ -1,75 +1,157 @@
 #!/usr/bin/env ruby
 
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/string/action'
 
 describe Puppet::String::Action do
   describe "when validating the action name" do
-    it "should require a name" do
-      lambda { Puppet::String::Action.new(nil,nil) }.should raise_error("'' is an invalid action name")
-    end
-
-    it "should not allow empty names" do
-      lambda { Puppet::String::Action.new(nil,'') }.should raise_error("'' is an invalid action name")
-    end
-
-    it "should not allow names with whitespace" do
-      lambda { Puppet::String::Action.new(nil,'foo bar') }.should raise_error("'foo bar' is an invalid action name")
-    end
-
-    it "should not allow names beginning with dashes" do
-      lambda { Puppet::String::Action.new(nil,'-foobar') }.should raise_error("'-foobar' is an invalid action name")
+    [nil, '', 'foo bar', '-foobar'].each do |input|
+      it "should treat #{input.inspect} as an invalid name" do
+        expect { Puppet::String::Action.new(nil, input) }.
+          should raise_error(/is an invalid action name/)
+      end
     end
   end
 
   describe "when invoking" do
     it "should be able to call other actions on the same object" do
       string = Puppet::String.new(:my_string, '0.0.1') do
         action(:foo) do
           invoke { 25 }
         end
 
         action(:bar) do
           invoke { "the value of foo is '#{foo}'" }
         end
       end
       string.foo.should == 25
       string.bar.should == "the value of foo is '25'"
     end
 
     # bar is a class action calling a class action
     # quux is a class action calling an instance action
     # baz is an instance action calling a class action
     # qux is an instance action calling an instance action
     it "should be able to call other actions on the same object when defined on a class" do
       class Puppet::String::MyStringBaseClass < Puppet::String
         action(:foo) do
           invoke { 25 }
         end
 
         action(:bar) do
           invoke { "the value of foo is '#{foo}'" }
         end
 
         action(:quux) do
           invoke { "qux told me #{qux}" }
         end
       end
 
       string = Puppet::String::MyStringBaseClass.new(:my_inherited_string, '0.0.1') do
         action(:baz) do
           invoke { "the value of foo in baz is '#{foo}'" }
         end
 
         action(:qux) do
           invoke { baz }
         end
       end
       string.foo.should  == 25
       string.bar.should  == "the value of foo is '25'"
       string.quux.should == "qux told me the value of foo in baz is '25'"
       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
+    it "should support options with an empty block" do
+      string = Puppet::String.new(:action_level_options, '0.0.1') do
+        action :foo do
+          option "--bar" do
+            # this line left deliberately blank
+          end
+        end
+      end
+
+      string.should_not be_option :bar
+      string.get_action(:foo).should be_option :bar
+    end
+
+    it "should return only action level options when there are no string options" do
+      string = Puppet::String.new(:action_level_options, '0.0.1') do
+        action :foo do option "--bar" end
+      end
+
+      string.get_action(:foo).options.should =~ [:bar]
+    end
+
+    describe "with both string and action options" do
+      let :string do
+        Puppet::String.new(:action_level_options, '0.0.1') do
+          action :foo do option "--bar" end
+          action :baz do option "--bim" end
+          option "--quux"
+        end
+      end
+
+      it "should return combined string and action options" do
+        string.get_action(:foo).options.should =~ [:bar, :quux]
+      end
+
+      it "should get an action option when asked" do
+        string.get_action(:foo).get_option(:bar).
+          should be_an_instance_of Puppet::String::Option
+      end
+
+      it "should get a string option when asked" do
+        string.get_action(:foo).get_option(:quux).
+          should be_an_instance_of Puppet::String::Option
+      end
+
+      it "should return options only for this action" do
+        string.get_action(:baz).options.should =~ [:bim, :quux]
+      end
+    end
+
+    it_should_behave_like "things that declare options" do
+      def add_options_to(&block)
+        string = Puppet::String.new(:with_options, '0.0.1') do
+          action(:foo, &block)
+        end
+        string.get_action(:foo)
+      end
+    end
+
+    it "should fail when a string option duplicates an action option" do
+      expect {
+        Puppet::String.new(:action_level_options, '0.0.1') do
+          option "--foo"
+          action :bar do option "--foo" end
+        end
+      }.should raise_error ArgumentError, /Option foo conflicts with existing option foo/i
+    end
   end
 end
diff --git a/spec/unit/string/indirector_spec.rb b/spec/unit/string/indirector_spec.rb
index 89306c416..cb85eaa05 100755
--- a/spec/unit/string/indirector_spec.rb
+++ b/spec/unit/string/indirector_spec.rb
@@ -1,55 +1,56 @@
 #!/usr/bin/env ruby
 
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/string/indirector'
 
 describe Puppet::String::Indirector do
-  before do
-    @instance = Puppet::String::Indirector.new(:test, '0.0.1')
-
-    @indirection = stub 'indirection', :name => :stub_indirection
-
-    @instance.stubs(:indirection).returns @indirection
+  subject do
+    instance = Puppet::String::Indirector.new(:test, '0.0.1')
+    indirection = stub('indirection',
+                       :name => :stub_indirection,
+                       :reset_terminus_class => nil)
+    instance.stubs(:indirection).returns indirection
+    instance
   end
 
   it "should be able to return a list of indirections" do
     Puppet::String::Indirector.indirections.should be_include("catalog")
   end
 
   it "should be able to return a list of terminuses for a given indirection" do
     Puppet::String::Indirector.terminus_classes(:catalog).should be_include("compiler")
   end
 
   describe "as an instance" do
     it "should be able to determine its indirection" do
       # Loading actions here an get, um, complicated
       Puppet::String.stubs(:load_actions)
       Puppet::String::Indirector.new(:catalog, '0.0.1').indirection.should equal(Puppet::Resource::Catalog.indirection)
     end
   end
 
   [:find, :search, :save, :destroy].each do |method|
     it "should define a '#{method}' action" do
       Puppet::String::Indirector.should be_action(method)
     end
 
-    it "should just call the indirection method when the '#{method}' action is invoked" do
-      @instance.indirection.expects(method).with(:test, "myargs")
-      @instance.send(method, :test, "myargs")
+    it "should call the indirection method when the '#{method}' action is invoked" do
+      subject.indirection.expects(method).with(:test, "myargs")
+      subject.send(method, :test, "myargs")
     end
   end
 
   it "should be able to override its indirection name" do
-    @instance.set_indirection_name :foo
-    @instance.indirection_name.should == :foo
+    subject.set_indirection_name :foo
+    subject.indirection_name.should == :foo
   end
 
   it "should be able to set its terminus class" do
-    @instance.indirection.expects(:terminus_class=).with(:myterm)
-    @instance.set_terminus(:myterm)
+    subject.indirection.expects(:terminus_class=).with(:myterm)
+    subject.set_terminus(:myterm)
   end
 
   it "should define a class-level 'info' action" do
     Puppet::String::Indirector.should be_action(:info)
   end
 end
diff --git a/spec/unit/string/option_builder_spec.rb b/spec/unit/string/option_builder_spec.rb
new file mode 100644
index 000000000..9e913c29c
--- /dev/null
+++ b/spec/unit/string/option_builder_spec.rb
@@ -0,0 +1,29 @@
+require 'puppet/string/option_builder'
+
+describe Puppet::String::OptionBuilder do
+  let :string do Puppet::String.new(:option_builder_testing, '0.0.1') end
+
+  it "should be able to construct an option without a block" do
+    Puppet::String::OptionBuilder.build(string, "--foo").
+      should be_an_instance_of Puppet::String::Option
+  end
+
+  describe "when using the DSL block" do
+    it "should work with an empty block" do
+      option = Puppet::String::OptionBuilder.build(string, "--foo") do
+        # This block deliberately left blank.
+      end
+
+      option.should be_an_instance_of Puppet::String::Option
+    end
+
+    it "should support documentation declarations" do
+      text = "this is the description"
+      option = Puppet::String::OptionBuilder.build(string, "--foo") do
+        desc text
+      end
+      option.should be_an_instance_of Puppet::String::Option
+      option.desc.should == text
+    end
+  end
+end
diff --git a/spec/unit/string/option_spec.rb b/spec/unit/string/option_spec.rb
new file mode 100644
index 000000000..f4f62ec37
--- /dev/null
+++ b/spec/unit/string/option_spec.rb
@@ -0,0 +1,75 @@
+require 'puppet/string/option'
+
+describe Puppet::String::Option do
+  let :string do Puppet::String.new(:option_testing, '0.0.1') end
+
+  describe "#optparse_to_name" do
+    ["", "=BAR", " BAR", "=bar", " bar"].each do |postfix|
+      { "--foo" => :foo, "-f" => :f }.each do |base, expect|
+        input = base + postfix
+        it "should map #{input.inspect} to #{expect.inspect}" do
+          option = Puppet::String::Option.new(string, input)
+          option.name.should == expect
+        end
+      end
+    end
+
+    [:foo, 12, nil, {}, []].each do |input|
+      it "should fail sensible when given #{input.inspect}" do
+        expect { Puppet::String::Option.new(string, input) }.
+          should raise_error ArgumentError, /is not valid for an option argument/
+      end
+    end
+
+    ["-foo", "-foo=BAR", "-foo BAR"].each do |input|
+      it "should fail with a single dash for long option #{input.inspect}" do
+        expect { Puppet::String::Option.new(string, input) }.
+          should raise_error ArgumentError, /long options need two dashes \(--\)/
+      end
+    end
+  end
+
+  it "requires a string when created" do
+    expect { Puppet::String::Option.new }.
+      should raise_error ArgumentError, /wrong number of arguments/
+  end
+
+  it "also requires some declaration arguments when created" do
+    expect { Puppet::String::Option.new(string) }.
+      should raise_error ArgumentError, /No option declarations found/
+  end
+
+  it "should infer the name from an optparse string" do
+    option = Puppet::String::Option.new(string, "--foo")
+    option.name.should == :foo
+  end
+
+  it "should infer the name when multiple optparse strings are given" do
+    option = Puppet::String::Option.new(string, "--foo", "-f")
+    option.name.should == :foo
+  end
+
+  it "should prefer the first long option name over a short option name" do
+    option = Puppet::String::Option.new(string, "-f", "--foo")
+    option.name.should == :foo
+  end
+
+  it "should create an instance when given a string and name" do
+    Puppet::String::Option.new(string, "--foo").
+      should be_instance_of Puppet::String::Option
+  end
+
+  describe "#to_s" do
+    it "should transform a symbol into a string" do
+      option = Puppet::String::Option.new(string, "--foo")
+      option.name.should == :foo
+      option.to_s.should == "foo"
+    end
+
+    it "should use - rather than _ to separate words in strings but not symbols" do
+      option = Puppet::String::Option.new(string, "--foo-bar")
+      option.name.should == :foo_bar
+      option.to_s.should == "foo-bar"
+    end
+  end
+end
diff --git a/spec/unit/string/string_collection_spec.rb b/spec/unit/string/string_collection_spec.rb
index 63ddf7c5e..184299e3c 100755
--- a/spec/unit/string/string_collection_spec.rb
+++ b/spec/unit/string/string_collection_spec.rb
@@ -1,184 +1,188 @@
 #!/usr/bin/env ruby
 
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'tmpdir'
 
 describe Puppet::String::StringCollection do
   before :all do
     @strings = subject.instance_variable_get("@strings")
     @strings_backup = @strings.dup
   end
 
   before { @strings.clear }
 
   after :all do
     subject.instance_variable_set("@strings", @strings_backup)
   end
 
   describe "::strings" do
   end
 
   describe "::validate_version" do
     it 'should permit three number versions' do
       subject.validate_version('10.10.10').should == true
     end
 
     it 'should permit versions with appended descriptions' do
       subject.validate_version('10.10.10beta').should == true
     end
 
     it 'should not permit versions with more than three numbers' do
       subject.validate_version('1.2.3.4').should == false
     end
 
     it 'should not permit versions with only two numbers' do
       subject.validate_version('10.10').should == false
     end
 
     it 'should not permit versions with only one number' do
       subject.validate_version('123').should == false
     end
 
     it 'should not permit versions with text in any position but at the end' do
       subject.validate_version('v1.1.1').should == false
     end
   end
 
   describe "::[]" do
     before :each do
       subject.instance_variable_get("@strings")[:foo]['0.0.1'] = 10
     end
 
     before :each do
       @dir = Dir.mktmpdir
       @lib = FileUtils.mkdir_p(File.join @dir, 'puppet', 'string')
       $LOAD_PATH.push(@dir)
     end
 
     after :each do
       FileUtils.remove_entry_secure @dir
       $LOAD_PATH.pop
     end
 
     it "should return the string with the given name" do
       subject["foo", '0.0.1'].should == 10
     end
 
     it "should attempt to load the string if it isn't found" do
       subject.expects(:require).with('puppet/string/bar')
       subject.expects(:require).with('bar@0.0.1/puppet/string/bar')
       subject["bar", '0.0.1']
     end
 
     it "should attempt to load the default string for the specified version :current" do
       subject.expects(:require).never # except...
       subject.expects(:require).with('puppet/string/fozzie')
       subject['fozzie', :current]
     end
   end
 
   describe "::string?" do
     before :each do
       subject.instance_variable_get("@strings")[:foo]['0.0.1'] = 10
     end
 
     it "should return true if the string specified is registered" do
       subject.string?("foo", '0.0.1').should == true
     end
 
     it "should attempt to require the string if it is not registered" do
       subject.expects(:require).with do |file|
         @strings[:bar]['0.0.1'] = true
         file == 'puppet/string/bar'
       end
       subject.string?("bar", '0.0.1').should == true
     end
 
     it "should return true if requiring the string registered it" do
       subject.stubs(:require).with do
         subject.instance_variable_get("@strings")[:bar]['0.0.1'] = 20
       end
     end
 
     it "should require the string by version if the 'current' version isn't it" do
-      subject.expects(:require).with('puppet/string/bar').raises(LoadError)
+      subject.expects(:require).with('puppet/string/bar').
+        raises(LoadError, 'no such file to load -- puppet/string/bar')
       subject.expects(:require).with do |file|
         @strings[:bar]['0.0.1'] = true
         file == 'bar@0.0.1/puppet/string/bar'
       end
       subject.string?("bar", '0.0.1').should == true
     end
 
     it "should return false if the string is not registered" do
       subject.stubs(:require).returns(true)
       subject.string?("bar", '0.0.1').should == false
     end
 
     it "should return false if there is a LoadError requiring the string" do
-      subject.stubs(:require).raises(LoadError)
+      subject.stubs(:require).
+        raises(LoadError, 'no such file to load -- puppet/string/bar').then.
+        raises(LoadError, 'no such file to load -- bar@0.0.1/puppet/string/bar')
       subject.string?("bar", '0.0.1').should == false
     end
 
     it "should register the version loaded by `:current` as `:current`" do
       subject.expects(:require).with do |file|
         @strings[:huzzah]['2.0.1'] = :huzzah_string
         file == 'puppet/string/huzzah'
       end
       subject.string?("huzzah", :current)
       @strings[:huzzah][:current].should == :huzzah_string
     end
 
     it "should register the version loaded from `puppet/string/{name}` as `:current`" do
       subject.expects(:require).with do |file|
         @strings[:huzzah]['2.0.1'] = :huzzah_string
         file == 'puppet/string/huzzah'
       end
       subject.string?("huzzah", '2.0.1')
       @strings[:huzzah][:current].should == :huzzah_string
     end
 
     it "should not register the version loaded from `{name}@{version}` as `:current`" do
-      subject.expects(:require).with('puppet/string/huzzah').raises(LoadError)
+      subject.expects(:require).with('puppet/string/huzzah').
+        raises(LoadError, 'no such file to load -- puppet/string/huzzah')
       subject.expects(:require).with do |file|
         @strings[:huzzah]['0.0.1'] = true
         file == 'huzzah@0.0.1/puppet/string/huzzah'
       end
       subject.string?("huzzah", '0.0.1')
       @strings[:huzzah].should_not have_key(:current)
     end
   end
 
   describe "::register" do
     it "should store the string by name" do
       string = Puppet::String.new(:my_string, '0.0.1')
       subject.register(string)
       subject.instance_variable_get("@strings").should == {:my_string => {'0.0.1' => string}}
     end
   end
 
   describe "::underscorize" do
     faulty = [1, "#foo", "$bar", "sturm und drang", :"sturm und drang"]
     valid  = {
       "Foo"      => :foo,
       :Foo       => :foo,
       "foo_bar"  => :foo_bar,
       :foo_bar   => :foo_bar,
       "foo-bar"  => :foo_bar,
       :"foo-bar" => :foo_bar,
     }
 
     valid.each do |input, expect|
       it "should map #{input.inspect} to #{expect.inspect}" do
         result = subject.underscorize(input)
         result.should == expect
       end
     end
 
     faulty.each do |input|
       it "should fail when presented with #{input.inspect} (#{input.class})" do
         expect { subject.underscorize(input) }.
           should raise_error ArgumentError, /not a valid string name/
       end
     end
   end
 end
diff --git a/spec/unit/string_spec.rb b/spec/unit/string_spec.rb
index 64d4f12f8..ddf855475 100755
--- a/spec/unit/string_spec.rb
+++ b/spec/unit/string_spec.rb
@@ -1,84 +1,145 @@
 #!/usr/bin/env ruby
 
 require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')
 
 describe Puppet::String do
   before :all do
     @strings = Puppet::String::StringCollection.instance_variable_get("@strings").dup
   end
 
   before :each do
     Puppet::String::StringCollection.instance_variable_get("@strings").clear
   end
 
   after :all do
     Puppet::String::StringCollection.instance_variable_set("@strings", @strings)
   end
 
   describe "#define" do
     it "should register the string" do
       string = Puppet::String.define(:string_test_register, '0.0.1')
       string.should == Puppet::String[:string_test_register, '0.0.1']
     end
 
     it "should load actions" do
       Puppet::String.any_instance.expects(:load_actions)
       Puppet::String.define(:string_test_load_actions, '0.0.1')
     end
 
     it "should require a version number" do
       proc { Puppet::String.define(:no_version) }.should raise_error(ArgumentError)
     end
   end
 
   describe "#initialize" do
     it "should require a version number" do
       proc { Puppet::String.new(:no_version) }.should raise_error(ArgumentError)
     end
 
     it "should require a valid version number" do
       proc { Puppet::String.new(:bad_version, 'Rasins') }.should raise_error(ArgumentError)
     end
 
     it "should instance-eval any provided block" do
-      face = Puppet::String.new(:string_test_block,'0.0.1') do
+      face = Puppet::String.new(:string_test_block, '0.0.1') do
         action(:something) do
           invoke { "foo" }
         end
       end
 
       face.something.should == "foo"
     end
   end
 
   it "should have a name" do
-    Puppet::String.new(:me,'0.0.1').name.should == :me
+    Puppet::String.new(:me, '0.0.1').name.should == :me
   end
 
   it "should stringify with its own name" do
-    Puppet::String.new(:me,'0.0.1').to_s.should =~ /\bme\b/
+    Puppet::String.new(:me, '0.0.1').to_s.should =~ /\bme\b/
   end
 
   it "should allow overriding of the default format" do
-    face = Puppet::String.new(:me,'0.0.1')
+    face = Puppet::String.new(:me, '0.0.1')
     face.set_default_format :foo
     face.default_format.should == :foo
   end
 
   it "should default to :pson for its format" do
     Puppet::String.new(:me, '0.0.1').default_format.should == :pson
   end
 
   # Why?
   it "should create a class-level autoloader" do
     Puppet::String.autoloader.should be_instance_of(Puppet::Util::Autoload)
   end
 
   it "should try to require strings that are not known" do
     Puppet::String::StringCollection.expects(:require).with "puppet/string/foo"
     Puppet::String::StringCollection.expects(:require).with "foo@0.0.1/puppet/string/foo"
     Puppet::String[:foo, '0.0.1']
   end
 
   it "should be able to load all actions in all search paths"
+
+
+  it_should_behave_like "things that declare options" do
+    def add_options_to(&block)
+      Puppet::String.new(:with_options, '0.0.1', &block)
+    end
+  end
+
+  describe "with string-level options" do
+    it "should not return any action-level options" do
+      string = Puppet::String.new(:with_options, '0.0.1') do
+        option "--foo"
+        option "--bar"
+        action :baz do
+          option "--quux"
+        end
+      end
+      string.options.should =~ [:foo, :bar]
+    end
+
+    it "should fail when a string option duplicates an action option" do
+      expect {
+        Puppet::String.new(:action_level_options, '0.0.1') do
+          action :bar do option "--foo" end
+          option "--foo"
+        end
+      }.should raise_error ArgumentError, /Option foo conflicts with existing option foo on/i
+    end
+
+    it "should work when two actions have the same option" do
+      string = Puppet::String.new(:with_options, '0.0.1') do
+        action :foo do option "--quux" end
+        action :bar do option "--quux" end
+      end
+
+      string.get_action(:foo).options.should =~ [:quux]
+      string.get_action(:bar).options.should =~ [:quux]
+    end
+  end
+
+  describe "with inherited options" do
+    let :string do
+      parent = Class.new(Puppet::String)
+      parent.option("--inherited")
+      string = parent.new(:example, '0.2.1')
+      string.option("--local")
+      string
+    end
+
+    describe "#options" do
+      it "should list inherited options" do
+        string.options.should =~ [:inherited, :local]
+      end
+    end
+
+    describe "#get_option" do
+      it "should return an inherited option object" do
+        string.get_option(:inherited).should be_an_instance_of Puppet::String::Option
+      end
+    end
+  end
 end