diff --git a/lib/puppet/application/string_base.rb b/lib/puppet/application/string_base.rb
index 6032e32f8..a082ba0e2 100644
--- a/lib/puppet/application/string_base.rb
+++ b/lib/puppet/application/string_base.rb
@@ -1,121 +1,120 @@
 require 'puppet/application'
 require 'puppet/string'
 
 class Puppet::Application::StringBase < 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
 
   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, :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
 
   # 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 preinit
     super
     trap(:INT) do
       $stderr.puts "Cancelling String"
       exit(0)
     end
 
     # We need to parse enough of the command line out early, to identify what
     # the action is, so that we can obtain the full set of options to parse.
 
     # 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
-    while (index += 1) < command_line.args.length do
+    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
-          if @string.get_option(option).takes_argument? then
-            # We don't validate if the argument is optional or mandatory,
-            # because it doesn't matter here.  We just assume that errors will
-            # be caught later. --daniel 2011-03-30
-            index += 1 unless command_line.args[index + 1] =~ /^-/
+          option = @string.get_option(option)
+          if option.takes_argument? 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
         command_line.args.delete_at(index)
       end
     end
 
     @action or raise ArgumentError, "No action given on the command line!"
 
     # Finally, we can interact with the default option code to build behaviour
     # around the full set of options we now know we support.
     @action.options.each do |option|
       option = @action.get_option(option) # make it the object.
       self.class.option(*option.optparse) # ...and make the CLI parse it.
     end
   end
 
   def setup
     Puppet::Util::Log.newdestination :console
 
     # 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
     @arguments = command_line.args
     @arguments << options
   end
 
 
   def main
     # Call the method associated with the provided action (e.g., 'find').
     if result = @string.send(@action.name, *arguments)
       puts render(result)
     end
     exit(exit_code)
   end
 end
diff --git a/lib/puppet/string/option.rb b/lib/puppet/string/option.rb
index e7b6f187c..f499e4b95 100644
--- a/lib/puppet/string/option.rb
+++ b/lib/puppet/string/option.rb
@@ -1,79 +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.
+  # 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.tr('-', '_')
+    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/spec/unit/application/string_base_spec.rb b/spec/unit/application/string_base_spec.rb
index 62869fe61..7f06c05f4 100755
--- a/spec/unit/application/string_base_spec.rb
+++ b/spec/unit/application/string_base_spec.rb
@@ -1,117 +1,136 @@
 #!/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
-  option("--[no-]foo")
 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[:basetest, '0.0.1'].action :foo do
-      option "--foo"
-      invoke { |*args| args.length }
+    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
 
   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 "#preinit" do
     before :each do
       app.command_line.stubs(:args).returns %w{}
     end
 
     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
 
         it "should set the string based on the type" do
           app.string.name.should == :basetest
         end
 
         it "should set the format based on the string default" do
           app.format.should == :pson
         end
 
         it "should find the action" do
           app.action.should be
           app.action.name.should == :foo
         end
       end
 
       it "should fail if no action is given" do
         expect { app.preinit }.
           should raise_error ArgumentError, /No action given/
       end
 
       it "should report a sensible error when options with = fail" do
-        app.command_line.stubs(:args).returns %w{--foo=bar foo}
+        app.command_line.stubs(:args).returns %w{--action=bar foo}
         expect { app.preinit }.
-          should raise_error ArgumentError, /Unknown option "--foo"/
+          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{--foo foo}
+        app.command_line.stubs(:args).returns %w{--action foo}
         expect { app.preinit }.
-          should raise_error ArgumentError, /Unknown option "--foo"/
+          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
     end
   end
 
   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 send the specified verb and name to the string" do
       app.string.expects(:foo).with(*app.arguments)
       app.main
     end
 
     it "should use its render method to render any result" do
       app.expects(:render).with(app.arguments.length + 1)
       app.main
     end
   end
 end