6 files changed, 100 insertions, 33 deletions
diff --git a/lib/puppet/string/action.rb b/lib/puppet/string/action.rb
index 4219aca0a..692e467b4 100644
--- a/lib/puppet/string/action.rb
+++ b/lib/puppet/string/action.rb
@@ -29,13 +29,19 @@ class Puppet::String::Action
   end
 
   def add_option(option)
-    if option? option.name then
-      raise ArgumentError, "#{option.name} duplicates an existing option on #{self}"
-    elsif @string.option? option.name then
-      raise ArgumentError, "#{option.name} duplicates an existing option on #{@string}"
+    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
 
-    @options[option.name] = option
+    option.aliases.each do |name|
+      @options[name] = option
+    end
+
+    option
   end
 
   def option?(name)
diff --git a/lib/puppet/string/action_builder.rb b/lib/puppet/string/action_builder.rb
index fb2a749ae..e76044470 100644
--- a/lib/puppet/string/action_builder.rb
+++ b/lib/puppet/string/action_builder.rb
@@ -23,8 +23,8 @@ class Puppet::String::ActionBuilder
     @action.invoke = block
   end
 
-  def option(name, attrs = {}, &block)
-    option = Puppet::String::OptionBuilder.build(@action, name, attrs, &block)
+  def option(*declaration, &block)
+    option = Puppet::String::OptionBuilder.build(@action, *declaration, &block)
     @action.add_option(option)
   end
 end
diff --git a/lib/puppet/string/indirector.rb b/lib/puppet/string/indirector.rb
index 48ec32680..71b1f3b12 100644
--- a/lib/puppet/string/indirector.rb
+++ b/lib/puppet/string/indirector.rb
@@ -2,6 +2,12 @@ require 'puppet'
 require 'puppet/string'
 
 class Puppet::String::Indirector < Puppet::String
+  warn "REVISIT: Need to redefine this to take arguments again, eh."
+  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
diff --git a/lib/puppet/string/option.rb b/lib/puppet/string/option.rb
index bdc3e07c5..26b769c2e 100644
--- a/lib/puppet/string/option.rb
+++ b/lib/puppet/string/option.rb
@@ -1,24 +1,69 @@
 class Puppet::String::Option
-  attr_reader :name, :string
+  attr_reader   :string
+  attr_reader   :name
+  attr_reader   :aliases
+  attr_accessor :desc
 
-  def initialize(string, name, attrs = {})
-    raise "#{name.inspect} is an invalid option name" unless name.to_s =~ /^[a-z]\w*$/
-    @string = string
-    @name   = name.to_sym
-    attrs.each do |k,v| send("#{k}=", v) end
+  def takes_argument?
+    !!@argument
+  end
+  def optional_argument?
+    !!@optional_argument
   end
 
+  def initialize(string, *declaration, &block)
+    @string   = string
+    @optparse = []
+
+    # Collect and sort the arguments in the declaration.
+    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
+      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.
   def to_s
     @name.to_s.tr('_', '-')
   end
 
-  Types = [:boolean, :string]
-  def type
-    @type ||= :boolean
-  end
-  def type=(input)
-    value = begin input.to_sym rescue nil end
-    Types.include?(value) or raise ArgumentError, "#{input.inspect} is not a valid type"
-    @type = value
+  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('-', '_')
+    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
index 2087cbc99..da0d213fb 100644
--- a/lib/puppet/string/option_builder.rb
+++ b/lib/puppet/string/option_builder.rb
@@ -3,14 +3,14 @@ require 'puppet/string/option'
 class Puppet::String::OptionBuilder
   attr_reader :option
 
-  def self.build(string, name, attrs = {}, &block)
-    new(string, name, attrs, &block).option
+  def self.build(string, *declaration, &block)
+    new(string, *declaration, &block).option
   end
 
   private
-  def initialize(string, name, attrs, &block)
+  def initialize(string, *declaration, &block)
     @string = string
-    @option = Puppet::String::Option.new(string, name, attrs)
+    @option = Puppet::String::Option.new(string, *declaration)
     block and instance_eval(&block)
     @option
   end
diff --git a/lib/puppet/string/option_manager.rb b/lib/puppet/string/option_manager.rb
index df3ae6b4b..f952ad4f0 100644
--- a/lib/puppet/string/option_manager.rb
+++ b/lib/puppet/string/option_manager.rb
@@ -3,16 +3,26 @@ 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(name, attrs = {}, &block)
-    @options ||= {}
-    raise ArgumentError, "Option #{name} already defined for #{self}" if option?(name)
-    actions.each do |action|
-      if get_action(action).option?(name) then
-        raise ArgumentError, "Option #{name} already defined on action #{action} for #{self}"
+  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 = Puppet::String::OptionBuilder.build(self, name, &block)
-    @options[option.name] = option
+
+    option.aliases.each { |name| @options[name] = option }
+    option
   end
 
   def options