From 822d5303f01b42cb074db52e6ee2c05e913ba9c5 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Fri, 22 Apr 2011 14:09:44 -0700
Subject: maint: clean up test headers on face spec files.

A whole pile of spec files for faces were not pulling in the regular
spec_helper, or the puppet/face library before they used it.  This worked fine
by coincidence when they ran together, but blew up if run separately.

Reviewed-By: Jesse Wolf <jesse@puppetlabs.com>
---
 lib/puppet/interface/option.rb | 2 ++
 1 file changed, 2 insertions(+)

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/option.rb b/lib/puppet/interface/option.rb
index f4c56cb2c..3d3840ff6 100644
--- a/lib/puppet/interface/option.rb
+++ b/lib/puppet/interface/option.rb
@@ -1,3 +1,5 @@
+require 'puppet/interface'
+
 class Puppet::Interface::Option
   def initialize(parent, *declaration, &block)
     @parent   = parent
-- 
cgit 


From bbf777f5f47b98d35fbbc7b8e3983d79af559017 Mon Sep 17 00:00:00 2001
From: Pieter van de Bruggen <pieter@puppetlabs.com>
Date: Tue, 26 Apr 2011 16:07:21 -0700
Subject: (#7249) Publicize ActionBuilder DSL methods.

This change permits users to call functions with a reference to
`self` that can augment the in-progress action declaration, which can
be helpful in some more involved cases.

Reviewed-By: Max Martin
Reviewed-By: Daniel Pittman
---
 lib/puppet/interface/action_builder.rb | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/action_builder.rb b/lib/puppet/interface/action_builder.rb
index 2ffa38709..afc49e945 100644
--- a/lib/puppet/interface/action_builder.rb
+++ b/lib/puppet/interface/action_builder.rb
@@ -9,13 +9,6 @@ class Puppet::Interface::ActionBuilder
     new(face, name, &block).action
   end
 
-  private
-  def initialize(face, name, &block)
-    @face   = face
-    @action = Puppet::Interface::Action.new(face, name)
-    instance_eval(&block)
-  end
-
   # Ideally the method we're defining here would be added to the action, and a
   # method on the face would defer to it, but we can't get scope correct, so
   # we stick with this. --daniel 2011-03-24
@@ -56,18 +49,25 @@ class Puppet::Interface::ActionBuilder
   # Metaprogram the simple DSL from the target class.
   Puppet::Interface::Action.instance_methods.grep(/=$/).each do |setter|
     next if setter =~ /^=/
-    dsl = setter.sub(/=$/, '')
+    property = setter.sub(/=$/, '')
 
-    unless private_instance_methods.include? dsl
+    unless public_instance_methods.include? property
       # Using eval because the argument handling semantics are less awful than
       # when we use the define_method/block version.  The later warns on older
       # Ruby versions if you pass the wrong number of arguments, but carries
       # on, which is totally not what we want. --daniel 2011-04-18
-      eval <<METHOD
-def #{dsl}(value)
-  @action.#{dsl} = value
-end
-METHOD
+      eval <<-METHOD
+        def #{property}(value)
+          @action.#{property} = value
+        end
+      METHOD
     end
   end
+
+  private
+  def initialize(face, name, &block)
+    @face   = face
+    @action = Puppet::Interface::Action.new(face, name)
+    instance_eval(&block)
+  end
 end
-- 
cgit 


From c627fad08c4866fbef300e887750c29963985635 Mon Sep 17 00:00:00 2001
From: Pieter van de Bruggen <pieter@puppetlabs.com>
Date: Tue, 26 Apr 2011 16:34:25 -0700
Subject: (#7251) Let exceptions raised in decorators rise.

This allows users to write before_action advice that does basic
option validation very easily.

Reviewed-By: Daniel Pittman
---
 lib/puppet/interface/action.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index 08bc0a345..464b2a738 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -239,7 +239,7 @@ WRAPPER
     end.select(&:required?).collect(&:name) - args.last.keys
 
     return if required.empty?
-    raise ArgumentError, "missing required options (#{required.join(', ')})"
+    raise ArgumentError, "The following options are required: #{required.join(', ')}"
   end
 
   ########################################################################
-- 
cgit 


From 59e7ef15507de48f6504ef21a8e0e775104961c6 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Tue, 26 Apr 2011 23:30:08 -0700
Subject: (#6962) Move documentation support into a module.

Given that we have identical documentation behaviour in the face and action
code, it should properly be written once.  So, move it into a module, extend
the other classes with it, and have done.
---
 lib/puppet/interface/action.rb         |  52 ++++++----
 lib/puppet/interface/action_manager.rb |   4 +-
 lib/puppet/interface/documentation.rb  | 168 +++++++++++++++++++++++++++++++++
 3 files changed, 207 insertions(+), 17 deletions(-)
 create mode 100644 lib/puppet/interface/documentation.rb

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index 08bc0a345..18c7ab057 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -1,12 +1,21 @@
-# -*- coding: utf-8 -*-
 require 'puppet/interface'
-require 'puppet/interface/option'
+require 'puppet/interface/documentation'
+require 'prettyprint'
 
 class Puppet::Interface::Action
+  include Puppet::Interface::DocSupport
+
   def initialize(face, name, attrs = {})
     raise "#{name.inspect} is an invalid action name" unless name.to_s =~ /^[a-z]\w*$/
     @face    = face
     @name    = name.to_sym
+
+    # The few bits of documentation we actually demand.  The default license
+    # is a favour to our end users; if you happen to get that in a core face
+    # report it as a bug, please. --daniel 2011-04-26
+    @authors = []
+    @license  = 'All Rights Reserved'
+
     attrs.each do |k, v| send("#{k}=", v) end
 
     @options        = {}
@@ -30,8 +39,31 @@ class Puppet::Interface::Action
     !!@default
   end
 
-  attr_accessor :summary
-
+  ########################################################################
+  # Documentation...
+  def synopsis
+    output = PrettyPrint.format do |s|
+      s.text("puppet #{@face.name}")
+      s.text(" #{name}") unless default?
+      s.breakable
+
+      options.each do |option|
+        option = get_option(option)
+        wrap = option.required? ? %w{ < > } : %w{ [ ] }
+
+        s.group(0, *wrap) do
+          option.optparse.each do |item|
+            unless s.current_group.first?
+              s.breakable
+              s.text '|'
+              s.breakable
+            end
+            s.text item
+          end
+        end
+      end
+    end
+  end
 
   ########################################################################
   # Support for rendering formats and all.
@@ -82,18 +114,6 @@ class Puppet::Interface::Action
   end
 
 
-  ########################################################################
-  # Documentation stuff, whee!
-  attr_accessor :summary, :description
-  def summary=(value)
-    value = value.to_s
-    value =~ /\n/ and
-      raise ArgumentError, "Face summary should be a single line; put the long text in 'description' instead."
-
-    @summary = value
-  end
-
-
   ########################################################################
   # Initially, this was defined to allow the @action.invoke pattern, which is
   # a very natural way to invoke behaviour given our introspection
diff --git a/lib/puppet/interface/action_manager.rb b/lib/puppet/interface/action_manager.rb
index 440be2d1b..c5eb8e08a 100644
--- a/lib/puppet/interface/action_manager.rb
+++ b/lib/puppet/interface/action_manager.rb
@@ -1,9 +1,11 @@
-require 'puppet/interface/action_builder'
+require 'puppet/interface/action'
 
 module Puppet::Interface::ActionManager
   # Declare that this app can take a specific action, and provide
   # the code to do so.
   def action(name, &block)
+    require 'puppet/interface/action_builder'
+
     @actions ||= {}
     @default_action ||= nil
     raise "Action #{name} already defined for #{self}" if action?(name)
diff --git a/lib/puppet/interface/documentation.rb b/lib/puppet/interface/documentation.rb
new file mode 100644
index 000000000..f3bf33da5
--- /dev/null
+++ b/lib/puppet/interface/documentation.rb
@@ -0,0 +1,168 @@
+class Puppet::Interface
+  module DocSupport
+    attr_accessor :summary
+    def summary(value = nil)
+      self.summary = value unless value.nil?
+      @summary
+    end
+    def summary=(value)
+      value = value.to_s
+      value =~ /\n/ and
+        raise ArgumentError, "Face summary should be a single line; put the long text in 'description' instead."
+
+      @summary = value
+    end
+
+    attr_accessor :description
+    def description(value = nil)
+      self.description = value unless value.nil?
+      @description
+    end
+
+    attr_accessor :examples
+    def examples(value = nil)
+      self.examples = value unless value.nil?
+      @examples
+    end
+
+    attr_accessor :short_description
+    def short_description(value = nil)
+      self.short_description = value unless value.nil?
+      if @short_description.nil? then
+        return nil if @description.nil?
+        lines = @description.split("\n")
+        grab  = [5, lines.index('') || 5].min
+        @short_description = lines[0, grab].join("\n")
+      end
+      @short_description
+    end
+
+    def author(value = nil)
+      unless value.nil? then
+        unless value.is_a? String
+          raise ArgumentError, 'author must be a string; use multiple statements for multiple authors'
+        end
+
+        if value =~ /\n/ then
+          raise ArgumentError, 'author should be a single line; use multiple statements for multiple authors'
+        end
+        @authors.push(value)
+      end
+      @authors.empty? ? nil : @authors.join("\n")
+    end
+    def author=(value)
+      if Array(value).any? {|x| x =~ /\n/ } then
+        raise ArgumentError, 'author should be a single line; use multiple statements'
+      end
+      @authors = Array(value)
+    end
+    def authors
+      @authors
+    end
+    def authors=(value)
+      if Array(value).any? {|x| x =~ /\n/ } then
+        raise ArgumentError, 'author should be a single line; use multiple statements'
+      end
+      @authors = Array(value)
+    end
+
+    attr_accessor :notes
+    def notes(value = nil)
+      @notes = value unless value.nil?
+      @notes
+    end
+
+    attr_accessor :license
+    def license(value = nil)
+      @license = value unless value.nil?
+      @license
+    end
+
+    def copyright(owner = nil, years = nil)
+      if years.nil? and not owner.nil? then
+        raise ArgumentError, 'copyright takes the owners names, then the years covered'
+      end
+      self.copyright_owner = owner unless owner.nil?
+      self.copyright_years = years unless years.nil?
+
+      if self.copyright_years or self.copyright_owner then
+        "Copyright #{self.copyright_years} by #{self.copyright_owner}"
+      else
+        "Unknown copyright owner and years."
+      end
+    end
+
+    attr_accessor :copyright_owner
+    def copyright_owner=(value)
+      case value
+      when String then @copyright_owner = value
+      when Array  then @copyright_owner = value.join(", ")
+      else
+        raise ArgumentError, "copyright owner must be a string or an array of strings"
+      end
+      @copyright_owner
+    end
+
+    attr_accessor :copyright_years
+    def copyright_years=(value)
+      years = munge_copyright_year value
+      years = (years.is_a?(Array) ? years : [years]).
+        sort_by do |x| x.is_a?(Range) ? x.first : x end
+
+      @copyright_years = years.map do |year|
+        if year.is_a? Range then
+          "#{year.first}-#{year.last}"
+        else
+          year
+        end
+      end.join(", ")
+    end
+
+    def munge_copyright_year(input)
+      case input
+      when Range then input
+      when Integer then
+        if input < 1970 then
+          fault = "before 1970"
+        elsif input > (future = Time.now.year + 2) then
+          fault = "after #{future}"
+        end
+        if fault then
+          raise ArgumentError, "copyright with a year #{fault} is very strange; did you accidentally add or subtract two years?"
+        end
+
+        input
+
+      when String then
+        input.strip.split(/,/).map do |part|
+          part = part.strip
+          if part =~ /^\d+$/ then
+            part.to_i
+          elsif found = part.split(/-/) then
+            unless found.length == 2 and found.all? {|x| x.strip =~ /^\d+$/ }
+              raise ArgumentError, "#{part.inspect} is not a good copyright year or range"
+            end
+            Range.new(found[0].to_i, found[1].to_i)
+          else
+            raise ArgumentError, "#{part.inspect} is not a good copyright year or range"
+          end
+        end
+
+      when Array then
+        result = []
+        input.each do |item|
+          item = munge_copyright_year item
+          if item.is_a? Array
+            result.concat item
+          else
+            result << item
+          end
+        end
+        result
+
+      else
+        raise ArgumentError, "#{input.inspect} is not a good copyright year, set, or range"
+      end
+    end
+  end
+end
-- 
cgit 


From e8eb290a1681baa19ef0b035af7cf17daadc6069 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Wed, 27 Apr 2011 10:05:48 -0700
Subject: (#6962) Finish documentation API on Face options.

This extends the last of the documentation support, down into options, so they
can be described as expected.  In the process we split out the modular docs
API into a full and short version   options only want short docs, but the
behaviours are identical to the full version.
---
 lib/puppet/interface/action.rb        | 2 +-
 lib/puppet/interface/documentation.rb | 6 +++++-
 lib/puppet/interface/option.rb        | 6 +++++-
 3 files changed, 11 insertions(+), 3 deletions(-)

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index 18c7ab057..177df81f2 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -3,7 +3,7 @@ require 'puppet/interface/documentation'
 require 'prettyprint'
 
 class Puppet::Interface::Action
-  include Puppet::Interface::DocSupport
+  include Puppet::Interface::FullDocs
 
   def initialize(face, name, attrs = {})
     raise "#{name.inspect} is an invalid action name" unless name.to_s =~ /^[a-z]\w*$/
diff --git a/lib/puppet/interface/documentation.rb b/lib/puppet/interface/documentation.rb
index f3bf33da5..d0bfbb261 100644
--- a/lib/puppet/interface/documentation.rb
+++ b/lib/puppet/interface/documentation.rb
@@ -1,5 +1,5 @@
 class Puppet::Interface
-  module DocSupport
+  module TinyDocs
     attr_accessor :summary
     def summary(value = nil)
       self.summary = value unless value.nil?
@@ -18,6 +18,10 @@ class Puppet::Interface
       self.description = value unless value.nil?
       @description
     end
+  end
+
+  module FullDocs
+    include TinyDocs
 
     attr_accessor :examples
     def examples(value = nil)
diff --git a/lib/puppet/interface/option.rb b/lib/puppet/interface/option.rb
index 3d3840ff6..493b5c3bd 100644
--- a/lib/puppet/interface/option.rb
+++ b/lib/puppet/interface/option.rb
@@ -1,6 +1,10 @@
 require 'puppet/interface'
 
 class Puppet::Interface::Option
+  include Puppet::Interface::FullDocs
+  # For compatibility, deprecated, and should go fairly soon...
+  ['', '='].each { |x| alias :"desc#{x}" :"description#{x}" }
+
   def initialize(parent, *declaration, &block)
     @parent   = parent
     @optparse = []
@@ -80,7 +84,7 @@ class Puppet::Interface::Option
   end
 
   attr_reader   :parent, :name, :aliases, :optparse
-  attr_accessor :required, :desc
+  attr_accessor :required
 
   attr_accessor :before_action
   def before_action=(proc)
-- 
cgit 


From 0256d67e1a51a37f2c87ec197bdff6ef3a6b269f Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Wed, 27 Apr 2011 10:38:41 -0700
Subject: (#6962) Add integration tests on Face documentation.

We now run all the faces, and their actions, as well as global help through
the wringer in this test: this way we can be confident that we have, at least,
the ability to generate the help without a user-visible failure.

We also check that we have set copyright and license terms in our own faces.
Theoretically this might fail if the end user has extra faces on LOAD_PATH,
but my hope is that we won't hit that...
---
 lib/puppet/interface/option.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/option.rb b/lib/puppet/interface/option.rb
index 493b5c3bd..b68bdeb12 100644
--- a/lib/puppet/interface/option.rb
+++ b/lib/puppet/interface/option.rb
@@ -1,7 +1,7 @@
 require 'puppet/interface'
 
 class Puppet::Interface::Option
-  include Puppet::Interface::FullDocs
+  include Puppet::Interface::TinyDocs
   # For compatibility, deprecated, and should go fairly soon...
   ['', '='].each { |x| alias :"desc#{x}" :"description#{x}" }
 
-- 
cgit 


From 69e4b1c5c024f4e6087054a7d8e77d30cacc62c8 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Wed, 27 Apr 2011 14:21:42 -0700
Subject: (#7122) Enforce call arity on actions in the CLI wrapper.

We had a problem, previously, in the generic translation of command line
arguments to Ruby method calls: we could mistake the options, added by the CLI
wrapper, for a positional argument to the action method.

This was caused by a combination of factors, but primarily that the wrapper
methods for actions are designed to present a friendly, helpful Ruby API for
internal use.  Consequently, they have a default value if you don't wish to
pass options.

Unfortunately, this meant that the options that the CLI *always* passed could
be treated as a positional argument instead, and the default set of options
added to the back of the call.

To resolve this we now check the number of positional arguments in the CLI
wrapper, and raise an exception if they are mismatched.  This makes the
generic CLI handling do the right thing in adapting the command line
arguments to the Ruby API.

(As an aside, we would have had a similar-but-different failure mode if we
 type-checked positional arguments: these calls would have failed with an
 invalid argument validation error.)

Reviewed-By: Max Martin <max@puppetlabs.com>
---
 lib/puppet/interface/action.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index ac66d2946..9c9741b52 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -168,11 +168,12 @@ class Puppet::Interface::Action
   # 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
+  attr_reader :positional_arg_count
   def when_invoked=(block)
 
     internal_name = "#{@name} implementation, required on Ruby 1.8".to_sym
 
-    arity = block.arity
+    arity = @positional_arg_count = block.arity
     if arity == 0 then
       # This will never fire on 1.8.7, which treats no arguments as "*args",
       # but will on 1.9.2, which treats it as "no arguments".  Which bites,
-- 
cgit 


From 207deae2dc06ca439e3b5ee9b044221a1c2899bb Mon Sep 17 00:00:00 2001
From: Pieter van de Bruggen <pieter@puppetlabs.com>
Date: Fri, 29 Apr 2011 12:18:12 -0700
Subject: (#7289) Specify order for option decorations.

`before_action` decorations should always resolve in resolution order
from most general (inherited from furthest away) to most specific
(declared on the instance), and should always execute Face-level
option decorations before action-level option decorations.

`after_action` decorations should execute in the opposite order.

Reviewed-By: Daniel Pittman
---
 lib/puppet/interface/action.rb         | 14 +++++++++-----
 lib/puppet/interface/option_manager.rb | 29 ++++++++++++++++++++---------
 2 files changed, 29 insertions(+), 14 deletions(-)

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index 464b2a738..96bb5b438 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -9,7 +9,10 @@ class Puppet::Interface::Action
     @name    = name.to_sym
     attrs.each do |k, v| send("#{k}=", v) end
 
-    @options        = {}
+    # @options collects the added options in the order they're declared.
+    # @options_hash collects the options keyed by alias for quick lookups.
+    @options        = []
+    @options_hash   = {}
     @when_rendering = {}
   end
 
@@ -211,22 +214,23 @@ WRAPPER
     end
 
     option.aliases.each do |name|
-      @options[name] = option
+      @options << name
+      @options_hash[name] = option
     end
 
     option
   end
 
   def option?(name)
-    @options.include? name.to_sym
+    @options_hash.include? name.to_sym
   end
 
   def options
-    (@options.keys + @face.options).sort
+    @face.options + @options
   end
 
   def get_option(name, with_inherited_options = true)
-    option = @options[name.to_sym]
+    option = @options_hash[name.to_sym]
     if option.nil? and with_inherited_options
       option = @face.get_option(name)
     end
diff --git a/lib/puppet/interface/option_manager.rb b/lib/puppet/interface/option_manager.rb
index d42359c07..326a91d92 100644
--- a/lib/puppet/interface/option_manager.rb
+++ b/lib/puppet/interface/option_manager.rb
@@ -8,6 +8,11 @@ module Puppet::Interface::OptionManager
   end
 
   def add_option(option)
+    # @options collects the added options in the order they're declared.
+    # @options_hash collects the options keyed by alias for quick lookups.
+    @options      ||= []
+    @options_hash ||= {}
+
     option.aliases.each do |name|
       if conflict = get_option(name) then
         raise ArgumentError, "Option #{option} conflicts with existing option #{conflict}"
@@ -21,25 +26,30 @@ module Puppet::Interface::OptionManager
       end
     end
 
-    option.aliases.each { |name| @options[name] = option }
-    option
+    option.aliases.each do |name|
+      @options << name
+      @options_hash[name] = option
+    end
+
+    return option
   end
 
   def options
-    @options ||= {}
-    result = @options.keys
+    result = (@options ||= [])
 
     if self.is_a?(Class) and superclass.respond_to?(:options)
-      result += superclass.options
+      result = superclass.options + result
     elsif self.class.respond_to?(:options)
-      result += self.class.options
+      result = self.class.options + result
     end
-    result.sort
+
+    return result
   end
 
   def get_option(name, with_inherited_options = true)
-    @options ||= {}
-    result = @options[name.to_sym]
+    @options_hash ||= {}
+
+    result = @options_hash[name.to_sym]
     if result.nil? and with_inherited_options then
       if self.is_a?(Class) and superclass.respond_to?(:get_option)
         result = superclass.get_option(name)
@@ -47,6 +57,7 @@ module Puppet::Interface::OptionManager
         result = self.class.get_option(name)
       end
     end
+
     return result
   end
 
-- 
cgit 


From 97ae812f0a67ef01daed4e9220981e2bc7c70603 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Fri, 29 Apr 2011 13:29:17 -0700
Subject: (#7248) Fail if two aliases of one option are passed...

From the command line, and other facades, it is fairly difficult to call an
action with two aliases of the same option in the invocation, but from the
Ruby API it would be relatively easy to achieve.

This is now checked, and raises an error, so that we don't accidentally have
strange or unusual behaviour coming out of the whole system.

Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
---
 lib/puppet/interface/action.rb | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index 9c9741b52..b842c2831 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -255,6 +255,20 @@ WRAPPER
   end
 
   def validate_args(args)
+    # Check for multiple aliases for the same option...
+    args.last.keys.each do |name|
+      # #7290: If this isn't actually an option, ignore it for now.  We should
+      # probably fail, but that wasn't our API, and I don't want to perturb
+      # behaviour this late in the RC cycle. --daniel 2011-04-29
+      if option = get_option(name) then
+        overlap = (option.aliases & args.last.keys)
+        unless overlap.length == 1 then
+          raise ArgumentError, "Multiple aliases for the same option passed: #{overlap.join(', ')}"
+        end
+      end
+    end
+
+    # Check for missing mandatory options.
     required = options.map do |name|
       get_option(name)
     end.select(&:required?).collect(&:name) - args.last.keys
-- 
cgit 


From 65b9a3c4f4e6830ed094d46381050dfa72c7eccd Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Fri, 29 Apr 2011 15:19:24 -0700
Subject: (#7221) Strip bad whitespace from face and action docs.

We now strip whitespace in face (and related) documentation in two places:

We strip any trailing whitespace on each line, just because.

We strip any leading indent, but not all leading whitespace, from the text.
That is, we strip the *minimum* amount of whitespace that we can take from
every line in the documentation without changing the overall content.

Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
---
 lib/puppet/interface/documentation.rb | 104 ++++++++++++++++++++--------------
 1 file changed, 63 insertions(+), 41 deletions(-)

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/documentation.rb b/lib/puppet/interface/documentation.rb
index d0bfbb261..91db0e074 100644
--- a/lib/puppet/interface/documentation.rb
+++ b/lib/puppet/interface/documentation.rb
@@ -1,35 +1,74 @@
+# This isn't usable outside Puppet::Interface; don't load it alone.
 class Puppet::Interface
-  module TinyDocs
-    attr_accessor :summary
-    def summary(value = nil)
-      self.summary = value unless value.nil?
-      @summary
+  module DocGen
+    def self.strip_whitespace(text)
+      text.gsub!(/[ \t\f]+$/, '')
+
+      # We need to identify an indent: the minimum number of whitespace
+      # characters at the start of any line in the text.
+      indent = text.each_line.map {|x| x.index(/[^\s]/) }.compact.min
+
+      if indent > 0 then
+        text.gsub!(/^[ \t\f]{0,#{indent}}/, '')
+      end
+
+      return text
     end
-    def summary=(value)
-      value = value.to_s
-      value =~ /\n/ and
-        raise ArgumentError, "Face summary should be a single line; put the long text in 'description' instead."
 
-      @summary = value
+    # The documentation attributes all have some common behaviours; previously
+    # we open-coded them across the set of six things, but that seemed
+    # wasteful - especially given that they were literally the same, and had
+    # the same bug hidden in them.
+    #
+    # This feels a bit like overkill, but at least the common code is common
+    # now. --daniel 2011-04-29
+    def attr_doc(name, &validate)
+      # Now, which form of the setter do we want, validated or not?
+      get_arg = "value.to_s"
+      if validate
+        define_method(:"_validate_#{name}", validate)
+        get_arg = "_validate_#{name}(#{get_arg})"
+      end
+
+      # We use module_eval, which I don't like much, because we can't have an
+      # argument to a block with a default value in Ruby 1.8, and I don't like
+      # the side-effects (eg: no argument count validation) of using blocks
+      # without as metheds.  When we are 1.9 only (hah!) you can totally
+      # replace this with some up-and-up define_method. --daniel 2011-04-29
+      module_eval(<<-EOT, __FILE__, __LINE__ + 1)
+        def #{name}(value = nil)
+          self.#{name} = value unless value.nil?
+          @#{name}
+        end
+
+        def #{name}=(value)
+          @#{name} = Puppet::Interface::DocGen.strip_whitespace(#{get_arg})
+        end
+      EOT
     end
+  end
+
+  module TinyDocs
+    extend Puppet::Interface::DocGen
 
-    attr_accessor :description
-    def description(value = nil)
-      self.description = value unless value.nil?
-      @description
+    attr_doc :summary do |value|
+      value =~ /\n/ and
+        raise ArgumentError, "Face summary should be a single line; put the long text in 'description' instead."
+      value
     end
+
+    attr_doc :description
   end
 
   module FullDocs
+    extend Puppet::Interface::DocGen
     include TinyDocs
 
-    attr_accessor :examples
-    def examples(value = nil)
-      self.examples = value unless value.nil?
-      @examples
-    end
+    attr_doc :examples
+    attr_doc :notes
+    attr_doc :license
 
-    attr_accessor :short_description
+    attr_doc :short_description
     def short_description(value = nil)
       self.short_description = value unless value.nil?
       if @short_description.nil? then
@@ -50,37 +89,20 @@ class Puppet::Interface
         if value =~ /\n/ then
           raise ArgumentError, 'author should be a single line; use multiple statements for multiple authors'
         end
-        @authors.push(value)
+        @authors.push(Puppet::Interface::DocGen.strip_whitespace(value))
       end
       @authors.empty? ? nil : @authors.join("\n")
     end
-    def author=(value)
-      if Array(value).any? {|x| x =~ /\n/ } then
-        raise ArgumentError, 'author should be a single line; use multiple statements'
-      end
-      @authors = Array(value)
-    end
     def authors
       @authors
     end
-    def authors=(value)
+    def author=(value)
       if Array(value).any? {|x| x =~ /\n/ } then
         raise ArgumentError, 'author should be a single line; use multiple statements'
       end
-      @authors = Array(value)
-    end
-
-    attr_accessor :notes
-    def notes(value = nil)
-      @notes = value unless value.nil?
-      @notes
-    end
-
-    attr_accessor :license
-    def license(value = nil)
-      @license = value unless value.nil?
-      @license
+      @authors = Array(value).map{|x| Puppet::Interface::DocGen.strip_whitespace(x) }
     end
+    alias :authors= :author=
 
     def copyright(owner = nil, years = nil)
       if years.nil? and not owner.nil? then
-- 
cgit 


From b23cc8abec1a1ec41b554b4e72f9a3c21feaf9da Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Fri, 29 Apr 2011 17:24:18 -0700
Subject: (#7282) action without `when_invoked` should fail...

We used to let actions be declared without the `when_invoked` block, which was
usually a sign of either someone writing their method code direct in action
declaration, or someone forgetting to add their code at all.

This was just let silently by: the error only showed up when you finally tried
to invoke the action, and a NoMethod error was raised by the face.

...except for our own testing.  We took advantage of this a whole pile of
times in there; fixing the original UI issue means fixing all those too.

Reviewed-By: Nick Lewis <nick@puppetlabs.com>
---
 lib/puppet/interface/action.rb         | 5 ++++-
 lib/puppet/interface/action_builder.rb | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index 0dbdd57bb..203d80827 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -171,7 +171,8 @@ class Puppet::Interface::Action
   # 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
-  attr_reader :positional_arg_count
+  attr_reader   :positional_arg_count
+  attr_accessor :when_invoked
   def when_invoked=(block)
 
     internal_name = "#{@name} implementation, required on Ruby 1.8".to_sym
@@ -219,9 +220,11 @@ WRAPPER
     if @face.is_a?(Class)
       @face.class_eval do eval wrapper, nil, file, line end
       @face.define_method(internal_name, &block)
+      @when_invoked = @face.instance_method(name)
     else
       @face.instance_eval do eval wrapper, nil, file, line end
       @face.meta_def(internal_name, &block)
+      @when_invoked = @face.method(name).unbind
     end
   end
 
diff --git a/lib/puppet/interface/action_builder.rb b/lib/puppet/interface/action_builder.rb
index afc49e945..0bf4f1408 100644
--- a/lib/puppet/interface/action_builder.rb
+++ b/lib/puppet/interface/action_builder.rb
@@ -69,5 +69,6 @@ class Puppet::Interface::ActionBuilder
     @face   = face
     @action = Puppet::Interface::Action.new(face, name)
     instance_eval(&block)
+    @action.when_invoked or raise ArgumentError, "actions need to know what to do when_invoked; please add the block"
   end
 end
-- 
cgit 


From 1b42725b5caab6f8e457e11fec2488fbe94e8e43 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Mon, 2 May 2011 15:14:25 -0700
Subject: (#7314) Faces fail horribly when one has a syntax error.

When we hit a syntax error in any face, a whole bunch of unrelated face things
would blow up in horrible ways.  Stack traces for all...

Now, instead, we catch that fault   but specifically only in the face file
and report it through our error logs, then quietly ignore the face.

Reviewed-By: Nick Lewis <nick@puppetlabs.com>
---
 lib/puppet/interface/face_collection.rb | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/face_collection.rb b/lib/puppet/interface/face_collection.rb
index 6e6afc545..baa424692 100644
--- a/lib/puppet/interface/face_collection.rb
+++ b/lib/puppet/interface/face_collection.rb
@@ -10,21 +10,12 @@ module Puppet::Interface::FaceCollection
     unless @loaded
       @loaded = true
       $LOAD_PATH.each do |dir|
-        next unless FileTest.directory?(dir)
-        Dir.chdir(dir) do
-          Dir.glob("puppet/face/*.rb").collect { |f| f.sub(/\.rb/, '') }.each do |file|
-            iname = file.sub(/\.rb/, '')
-            begin
-              require iname
-            rescue Exception => detail
-              puts detail.backtrace if Puppet[:trace]
-              raise "Could not load #{iname} from #{dir}/#{file}: #{detail}"
-            end
-          end
-        end
+        Dir.glob("#{dir}/puppet/face/*.rb").
+          collect {|f| File.basename(f, '.rb') }.
+          each {|name| self[name, :current] }
       end
     end
-    return @faces.keys.select {|name| @faces[name].length > 0 }
+    @faces.keys.select {|name| @faces[name].length > 0 }
   end
 
   def self.validate_version(version)
@@ -124,6 +115,10 @@ module Puppet::Interface::FaceCollection
     rescue LoadError => e
       raise unless e.message =~ %r{-- puppet/face/#{name}$}
       # ...guess we didn't find the file; return a much better problem.
+    rescue SyntaxError => e
+      raise unless e.message =~ %r{puppet/face/#{name}\.rb:\d+: }
+      Puppet.err "Failed to load face #{name}:\n#{e}"
+      # ...but we just carry on after complaining.
     end
 
     return get_face(name, version)
-- 
cgit 


From 0d6ac040f4223725586c2b5d71ffcb0d36206080 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Tue, 3 May 2011 13:33:43 -0700
Subject: maint: remove emacs 'coding' cookie from files.

This is unnecessary, and only turned up because Matz &c. impose their taste on
the rest of the world through the Emacs Ruby mode.  Since people are starting
to clone that, and it doesn't add value, eliminate it everywhere.

Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
---
 lib/puppet/interface/face_collection.rb | 1 -
 1 file changed, 1 deletion(-)

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/face_collection.rb b/lib/puppet/interface/face_collection.rb
index baa424692..12d3c56b1 100644
--- a/lib/puppet/interface/face_collection.rb
+++ b/lib/puppet/interface/face_collection.rb
@@ -1,4 +1,3 @@
-# -*- coding: utf-8 -*-
 require 'puppet/interface'
 
 module Puppet::Interface::FaceCollection
-- 
cgit 


From 5490f7aee19f477914520f0735858f58136122e4 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Tue, 3 May 2011 14:38:57 -0700
Subject: (#6962) Added 'returns' block to action documentation.

We want to be able to document the data returned from an action, since this is
one of the most critical parts of the API for Ruby developers.  This adds a
multiline documentation block that captures that.

Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
---
 lib/puppet/interface/action.rb | 2 ++
 1 file changed, 2 insertions(+)

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index 203d80827..d4cf23fce 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -3,6 +3,7 @@ require 'puppet/interface/documentation'
 require 'prettyprint'
 
 class Puppet::Interface::Action
+  extend Puppet::Interface::DocGen
   include Puppet::Interface::FullDocs
 
   def initialize(face, name, attrs = {})
@@ -44,6 +45,7 @@ class Puppet::Interface::Action
 
   ########################################################################
   # Documentation...
+  attr_doc :returns
   def synopsis
     output = PrettyPrint.format do |s|
       s.text("puppet #{@face.name}")
-- 
cgit 


From 8f81f56e8315a62e6c8ff8943c64df7594855359 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Tue, 3 May 2011 15:51:02 -0700
Subject: (#7326) Fix faces on Ruby 1.8.5

Turns out that String#each_line on Ruby 1.8.5 only takes a block, and will not
return an enumerable.  Rewrite to use split(/\n/) which has the same effect
but works on all our platforms.
---
 lib/puppet/interface/documentation.rb | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/documentation.rb b/lib/puppet/interface/documentation.rb
index 91db0e074..48e9a8b1a 100644
--- a/lib/puppet/interface/documentation.rb
+++ b/lib/puppet/interface/documentation.rb
@@ -6,7 +6,10 @@ class Puppet::Interface
 
       # We need to identify an indent: the minimum number of whitespace
       # characters at the start of any line in the text.
-      indent = text.each_line.map {|x| x.index(/[^\s]/) }.compact.min
+      #
+      # Using split rather than each_line is because the later only takes a
+      # block on Ruby 1.8.5 / Centos, and we support that. --daniel 2011-05-03
+      indent = text.split(/\n/).map {|x| x.index(/[^\s]/) }.compact.min
 
       if indent > 0 then
         text.gsub!(/^[ \t\f]{0,#{indent}}/, '')
-- 
cgit 


From 18b3584e16515cfc45aeaa8d0913de8e8bcb3e95 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Mon, 2 May 2011 16:27:28 -0700
Subject: (#7329) Consistent naming for rendering formats and hooks.

We refer to rendering formats pretty consistently as `json`, `yaml`, `s`, and
so forth; unqualified names.

On the other hand, we refer to the rendering hooks *mostly* as `to_*`, except
the `:for_humans` and `:json` formats.  Which is kind of confusing because of
the internal inconsistency, and because it doesn't match the public name.

Fix the code to resolve both, so the `to_*` format still works, but we mostly
expect to use the `*` version, to match public expectation.
---
 lib/puppet/interface/action.rb | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index d4cf23fce..622371a4e 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -76,8 +76,15 @@ class Puppet::Interface::Action
     unless type.is_a? Symbol
       raise ArgumentError, "The rendering format must be a symbol, not #{type.class.name}"
     end
-    return unless @when_rendering.has_key? type
-    return @when_rendering[type].bind(@face)
+    # Do we have a rendering hook for this name?
+    return @when_rendering[type].bind(@face) if @when_rendering.has_key? type
+
+    # How about by another name?
+    alt = type.to_s.sub(/^to_/, '').to_sym
+    return @when_rendering[alt].bind(@face) if @when_rendering.has_key? alt
+
+    # Guess not, nothing to run.
+    return nil
   end
   def set_rendering_method_for(type, proc)
     unless proc.is_a? Proc
-- 
cgit 


From dda32647b4c11ecb352e469088f2438835ff99d7 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Wed, 4 May 2011 10:40:55 -0700
Subject: (#7353) Note the :for_humans compatibility issue.

Where we need special support for :for_humans as an alias for :console, call
it out in comments.  This makes it clear to someone who wonders why what the
actual underlying purpose of the whole thing is.

Reviewed-By: Jacob Helwig <jacob@puppetlabs.com>
---
 lib/puppet/interface/action_builder.rb | 2 ++
 1 file changed, 2 insertions(+)

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/action_builder.rb b/lib/puppet/interface/action_builder.rb
index 0bf4f1408..16305530a 100644
--- a/lib/puppet/interface/action_builder.rb
+++ b/lib/puppet/interface/action_builder.rb
@@ -38,6 +38,8 @@ class Puppet::Interface::ActionBuilder
   def render_as(value = nil)
     value.nil? and raise ArgumentError, "You must give a rendering format to render_as"
 
+    # :for_humans is a compatibility alias for :console, but since we shipped
+    # it in 2.7.0rc1 we need to support it ongoing. --daniel 2011-05-04
     formats = Puppet::Network::FormatHandler.formats << :for_humans
     unless formats.include? value
       raise ArgumentError, "#{value.inspect} is not a valid rendering format: #{formats.sort.join(", ")}"
-- 
cgit 


From 61edff9ba5e8e4f8db8bb9cd5a84a986b8703c5d Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Thu, 5 May 2011 16:24:26 -0700
Subject: (#7353) Remove :for_humans format entirely.

Since we never shipped this in a real release, we don't need to maintain
compatibility.  So, remove it entirely from the codebase.

Reviewed-By: Max Martin <max@puppetlabs.com>
---
 lib/puppet/interface/action_builder.rb | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/action_builder.rb b/lib/puppet/interface/action_builder.rb
index 16305530a..62db8de06 100644
--- a/lib/puppet/interface/action_builder.rb
+++ b/lib/puppet/interface/action_builder.rb
@@ -38,9 +38,7 @@ class Puppet::Interface::ActionBuilder
   def render_as(value = nil)
     value.nil? and raise ArgumentError, "You must give a rendering format to render_as"
 
-    # :for_humans is a compatibility alias for :console, but since we shipped
-    # it in 2.7.0rc1 we need to support it ongoing. --daniel 2011-05-04
-    formats = Puppet::Network::FormatHandler.formats << :for_humans
+    formats = Puppet::Network::FormatHandler.formats
     unless formats.include? value
       raise ArgumentError, "#{value.inspect} is not a valid rendering format: #{formats.sort.join(", ")}"
     end
-- 
cgit