Merge branch 'master' of git://reductivelabs.com/puppet

4 files changed, 139 insertions, 36 deletions

diff --git a/CHANGELOG b/CHANGELOG
index f51e1f3c6..27aa0fa5e 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,4 +1,6 @@
 0.23.2 (misspiggy)
+    Added shortname support to config.rb and refactored addargs
+
     Fixed the problem in cron jobs where environment settings
     tended to multiple. (#749)
 
diff --git a/lib/puppet/util/config.rb b/lib/puppet/util/config.rb
index 6d42f0ea6..932314215 100644
--- a/lib/puppet/util/config.rb
+++ b/lib/puppet/util/config.rb
@@ -1,6 +1,8 @@
 require 'puppet'
 require 'sync'
 require 'puppet/transportable'
+require 'getoptlong'
+
 
 # The class for handling configuration files.
 class Puppet::Util::Config
@@ -67,19 +69,12 @@ class Puppet::Util::Config
     # Generate the list of valid arguments, in a format that GetoptLong can
     # understand, and add them to the passed option list.
     def addargs(options)
-        require 'getoptlong'
-
         # Hackish, but acceptable.  Copy the current ARGV for restarting.
         Puppet.args = ARGV.dup
 
         # Add all of the config parameters as valid options.
-        self.each { |param, obj|
-            if self.boolean?(param)
-                options << ["--#{param}", GetoptLong::NO_ARGUMENT]
-                options << ["--no-#{param}", GetoptLong::NO_ARGUMENT]
-            else
-                options << ["--#{param}", GetoptLong::REQUIRED_ARGUMENT]
-            end
+        self.each { |name, element|
+            element.getopt_args.each { |args| options << args }
         }
 
         return options
@@ -195,10 +190,17 @@ class Puppet::Util::Config
         @config.include?(name)
     end
 
+    # check to see if a short name is already defined
+    def shortinclude?(short)
+        short = short.intern if name.is_a? String
+        @shortnames.include?(short)
+    end
+
     # Create a new config object
     def initialize
         @order = []
         @config = {}
+        @shortnames = {}
 
         @created = []
         @returned = {}
@@ -499,7 +501,14 @@ class Puppet::Util::Config
             if @config.include?(name)
                 raise Puppet::Error, "Parameter %s is already defined" % name
             end
-            @config[name] = newelement(hash)
+            tryconfig = newelement(hash)
+            if short = tryconfig.short
+                if other = @shortnames[short]
+                    raise ArgumentError, "Parameter %s is already using short name '%s'" % [other.name, short]
+                end
+                @shortnames[short] = tryconfig
+            end
+            @config[name] = tryconfig
         }
     end
 
@@ -858,7 +867,7 @@ Generated on #{Time.now}.
     # The base element type.
     class CElement
         attr_accessor :name, :section, :default, :parent, :setbycli
-        attr_reader :desc
+        attr_reader :desc, :short
 
         # Unset any set value.
         def clear
@@ -885,6 +894,15 @@ Generated on #{Time.now}.
             @desc = value.gsub(/^\s*/, '')
         end
 
+        # get the arguments in getopt format
+        def getopt_args
+            if short
+                [["--#{name}", "-#{short}", GetoptLong::REQUIRED_ARGUMENT]]
+            else
+                [["--#{name}", GetoptLong::REQUIRED_ARGUMENT]]
+            end
+        end
+
         def hook=(block)
             meta_def :handle, &block
         end
@@ -929,6 +947,14 @@ Generated on #{Time.now}.
             end
         end
 
+        # short name for the celement
+        def short=(value)
+            if value.to_s.length != 1
+                raise ArgumentError, "Short names can only be one character."
+            end
+            @short = value.to_s
+        end
+
         # Convert the object to a config statement.
         def to_config
             str = @desc.gsub(/^/, "# ") + "\n"
@@ -1101,6 +1127,17 @@ Generated on #{Time.now}.
 
     # A simple boolean.
     class CBoolean < CElement
+        # get the arguments in getopt format
+        def getopt_args
+            if short
+                [["--#{name}", "-#{short}", GetoptLong::NO_ARGUMENT],
+                 ["--no-#{name}", GetoptLong::NO_ARGUMENT]]
+            else
+                [["--#{name}", GetoptLong::NO_ARGUMENT],
+                 ["--no-#{name}", GetoptLong::NO_ARGUMENT]]
+            end
+        end
+
         def munge(value)
             case value
             when true, "true": return true
diff --git a/lib/puppet/util/suidmanager.rb b/lib/puppet/util/suidmanager.rb
index e15656d77..c1ff66248 100644
--- a/lib/puppet/util/suidmanager.rb
+++ b/lib/puppet/util/suidmanager.rb
@@ -1,5 +1,6 @@
 require 'facter'
 require 'puppet/util/warnings'
+require 'forwardable'
 
 module Puppet::Util::SUIDManager
     include Puppet::Util::Warnings
diff --git a/test/util/config.rb b/test/util/config.rb
index 71343556f..9a1017058 100755
--- a/test/util/config.rb
+++ b/test/util/config.rb
@@ -10,6 +10,8 @@ require 'puppettest/parsertesting'
 class TestConfig < Test::Unit::TestCase
 	include PuppetTest
 	include PuppetTest::ParserTesting
+    CElement = Puppet::Util::Config::CElement
+    CBoolean = Puppet::Util::Config::CBoolean
 
     def setup
         super
@@ -535,34 +537,46 @@ yay = /a/path
         }
     end
 
-    def test_argadding
-        c = mkconfig
-
-        assert_nothing_raised {
-            @config.setdefaults("testing",
-                :onboolean => [true, "An on bool"],
-                :offboolean => [false, "An off bool"],
-                :string => ["a string", "A string arg"],
-                :file => ["/path/to/file", "A file arg"]
-            )
-        }
-        options = []
-
-        @config.addargs(options)
+    def test_addargs
+        @config.setdefaults("testing",
+                            :onboolean => [true, "An on bool"],
+                            :offboolean => [false, "An off bool"],
+                            :string => ["a string", "A string arg"],
+                            :file => ["/path/to/file", "A file arg"]
+                            )
+
+        should = []
+        @config.each { |name, element|
+            element.expects(:getopt_args).returns([name])
+            should << name
+        }
+        result = []
+        assert_nothing_raised("Add args failed") do
+            @config.addargs(result)
+        end
+        assert_equal(should, result, "Did not call addargs correctly.")
 
-        @config.each { |param, obj|
-            opt = "--%s" % param
-            assert(options.find { |ary|
-                ary[0] == opt
-            }, "Argument %s was not added" % opt)
+    end
 
-            if @config.boolean?(param)
-                o = "--no-%s" % param
-                assert(options.find { |ary|
-                ary[0] == o
-                }, "Boolean off %s was not added" % o)
+    def test_addargs_functional
+        @config.setdefaults("testing",
+                            :onboolean => [true, "An on bool"],
+                            :string => ["a string", "A string arg"]
+                            )
+        result = []
+        should = []
+        assert_nothing_raised("Add args failed") do
+            @config.addargs(result)
+        end
+        @config.each do |name, element|
+            if name == :onboolean
+                should << ["--onboolean", GetoptLong::NO_ARGUMENT]
+                should << ["--no-onboolean", GetoptLong::NO_ARGUMENT]
+            elsif name == :string
+                should << ["--string", GetoptLong::REQUIRED_ARGUMENT]
             end
-        }
+        end
+        assert_equal(should, result, "Add args functional test failed")
     end
 
     def test_usesection
@@ -1243,6 +1257,55 @@ inttest = 27
         # And make sure other params are unchanged
         assert_equal("unval", @config[:unchanged], "Unchanged value has somehow changed")
     end
+
+    # Test to make sure that we can set and get a short name
+    def test_celement_short_name
+        element = nil
+        assert_nothing_raised("Could not create celement") do
+            element = CElement.new :short => "n", :desc => "anything"
+        end
+        assert_equal("n", element.short, "Short value is not retained")
+
+        assert_raise(ArgumentError,"Allowed multicharactered short names.") do
+            element = CElement.new :short => "no", :desc => "anything"
+        end
+    end
+
+    # Test to make sure that no two celements have the same short name
+    def test_celement_short_name_not_duplicated
+        config = mkconfig
+        assert_nothing_raised("Could not create celement with short name.") do
+            config.setdefaults(:main,
+                               :one => { :default => "blah", :desc => "anything", :short => "o" })
+        end
+        assert_nothing_raised("Could not create second celement with short name.") do
+            config.setdefaults(:main,
+                               :two => { :default => "blah", :desc => "anything", :short => "i" })
+        end
+        assert_raise(ArgumentError, "Could create second celement with duplicate short name.") do
+            config.setdefaults(:main,
+                               :three => { :default => "blah", :desc => "anything", :short => "i" })
+        end
+        # make sure that when the above raises an expection that the config is not included
+        assert(!config.include?(:three), "Invalid configuration item was retained")
+    end
+
+    # Tell getopt which arguments are valid
+    def test_get_getopt_args
+        element = CElement.new :name => "foo", :desc => "anything"
+        assert_equal([["--foo", GetoptLong::REQUIRED_ARGUMENT]], element.getopt_args, "Did not produce appropriate getopt args")
+        
+        element.short = "n"
+        assert_equal([["--foo", "-n", GetoptLong::REQUIRED_ARGUMENT]], element.getopt_args, "Did not produce appropriate getopt args")
+
+        element = CBoolean.new :name => "foo", :desc => "anything"
+        assert_equal([["--foo", GetoptLong::NO_ARGUMENT], ["--no-foo", GetoptLong::NO_ARGUMENT]],
+                     element.getopt_args, "Did not produce appropriate getopt args")
+
+        element.short = "n"
+        assert_equal([["--foo", "-n", GetoptLong::NO_ARGUMENT],["--no-foo", GetoptLong::NO_ARGUMENT]],
+                      element.getopt_args, "Did not produce appropriate getopt args")
+    end
 end
 
 # $Id$