From dca1f077dd7a818aee447222a7649742f2b1575f Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Thu, 14 Apr 2011 15:20:38 -0700
Subject: (#6978) Add before and after decorators to actions from options.

Options can now add before_action and after_action blocks; these are invoked
before or after any action is invoked on the face.  This allows these options
to declare common behaviour and have it automatically applied to the actions
invoked.

Option hooks have no defined order of invocation: they will run in a
completely random order.  Where there are dependencies they should be on the
value of the options hash passed to the invocation, not on side-effects of the
other invocations.

You are not able to influence the arguments, options, or calling of the action
body in a before or after decorator.  This is by design.

The invocation passes to the hook:

 1. The action object representing this action.
 2. The arguments to the action, as an array.
 3. The options for the action, as a hash.

Paired-With: Max Martin <max@puppetlabs.com>
---
 lib/puppet/interface/action.rb         | 39 ++++++++++++++++++++++------
 lib/puppet/interface/option.rb         | 47 ++++++++++++++++++++++++----------
 lib/puppet/interface/option_builder.rb | 25 +++++++++++++++---
 lib/puppet/interface/option_manager.rb |  4 +--
 4 files changed, 89 insertions(+), 26 deletions(-)

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index db338e39e..3946b743e 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -7,8 +7,9 @@ class Puppet::Interface::Action
     raise "#{name.inspect} is an invalid action name" unless name.to_s =~ /^[a-z]\w*$/
     @face    = face
     @name    = name.to_sym
-    @options = {}
     attrs.each do |k, v| send("#{k}=", v) end
+
+    @options = {}
   end
 
   # This is not nice, but it is the easiest way to make us behave like the
@@ -84,11 +85,21 @@ class Puppet::Interface::Action
     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"
+    wrapper = <<WRAPPER
+def #{@name}(*args, &block)
+  if args.last.is_a? Hash then
+    options = args.last
+  else
+    args << (options = {})
+  end
+
+  action = get_action(#{name.inspect})
+  __invoke_decorations(:before, action, args, options)
+  rval = self.__send__(#{internal_name.inspect}, *args)
+  __invoke_decorations(:after, action, args, options)
+  return rval
+end
+WRAPPER
 
     if @face.is_a?(Class)
       @face.class_eval do eval wrapper, nil, file, line end
@@ -123,7 +134,19 @@ class Puppet::Interface::Action
     (@options.keys + @face.options).sort
   end
 
-  def get_option(name)
-    @options[name.to_sym] || @face.get_option(name)
+  def get_option(name, with_inherited_options = true)
+    option = @options[name.to_sym]
+    if option.nil? and with_inherited_options
+      option = @face.get_option(name)
+    end
+    option
+  end
+
+  ########################################################################
+  # Support code for action decoration; see puppet/interface.rb for the gory
+  # details of why this is hidden away behind private. --daniel 2011-04-15
+  private
+  def __decorate(type, name, proc)
+    @face.__send__ :__decorate, type, name, proc
   end
 end
diff --git a/lib/puppet/interface/option.rb b/lib/puppet/interface/option.rb
index ccc2fbba7..2abcd4033 100644
--- a/lib/puppet/interface/option.rb
+++ b/lib/puppet/interface/option.rb
@@ -1,19 +1,6 @@
 require 'puppet/interface'
 
 class Puppet::Interface::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 = []
@@ -79,4 +66,38 @@ class Puppet::Interface::Option
     raise "#{name.inspect} is an invalid option name" unless name.to_s =~ /^[a-z]\w*$/
     name.to_sym
   end
+
+
+  def takes_argument?
+    !!@argument
+  end
+  def optional_argument?
+    !!@optional_argument
+  end
+
+
+  attr_reader   :parent, :name, :aliases, :optparse
+  attr_accessor :desc
+
+  attr_accessor :before_action
+  def before_action=(proc)
+    proc.is_a? Proc or raise ArgumentError, "before action hook for #{self} is a #{proc.class.name.inspect}, not a proc"
+    @before_action =
+      @parent.__send__(:__decorate, :before, __decoration_name(:before), proc)
+  end
+
+  attr_accessor :after_action
+  def after_action=(proc)
+    proc.is_a? Proc or raise ArgumentError, "after action hook for #{self} is a #{proc.class.name.inspect}, not a proc"
+    @after_action =
+      @parent.__send__(:__decorate, :after, __decoration_name(:after), proc)
+  end
+
+  def __decoration_name(type)
+    if @parent.is_a? Puppet::Interface::Action then
+      :"option #{name} from #{parent.name} #{type} decoration"
+    else
+      :"option #{name} #{type} decoration"
+    end
+  end
 end
diff --git a/lib/puppet/interface/option_builder.rb b/lib/puppet/interface/option_builder.rb
index 2240b3e4a..ccad0850d 100644
--- a/lib/puppet/interface/option_builder.rb
+++ b/lib/puppet/interface/option_builder.rb
@@ -17,9 +17,28 @@ class Puppet::Interface::OptionBuilder
 
   # Metaprogram the simple DSL from the option class.
   Puppet::Interface::Option.instance_methods.grep(/=$/).each do |setter|
-    next if setter =~ /^=/      # special case, darn it...
+    next if setter =~ /^=/
+    dsl = setter.sub(/=$/, '')
 
-    dsl = setter.to_s.sub(/=$/, '')
-    define_method(dsl) do |value| @option.send(setter, value) end
+    unless self.class.methods.include?(dsl)
+      define_method(dsl) do |value| @option.send(setter, value) end
+    end
+  end
+
+  # Override some methods that deal in blocks, not objects.
+  def before_action(&block)
+    block or raise ArgumentError, "#{@option} before_action requires a block"
+    if @option.before_action
+      raise ArgumentError, "#{@option} already has a before_action set"
+    end
+    @option.before_action = block
+  end
+
+  def after_action(&block)
+    block or raise ArgumentError, "#{@option} after_action requires a block"
+    if @option.after_action
+      raise ArgumentError, "#{@option} already has a after_action set"
+    end
+    @option.after_action = block
   end
 end
diff --git a/lib/puppet/interface/option_manager.rb b/lib/puppet/interface/option_manager.rb
index 56df9760f..d42359c07 100644
--- a/lib/puppet/interface/option_manager.rb
+++ b/lib/puppet/interface/option_manager.rb
@@ -37,10 +37,10 @@ module Puppet::Interface::OptionManager
     result.sort
   end
 
-  def get_option(name)
+  def get_option(name, with_inherited_options = true)
     @options ||= {}
     result = @options[name.to_sym]
-    unless result then
+    if result.nil? and with_inherited_options then
       if self.is_a?(Class) and superclass.respond_to?(:get_option)
         result = superclass.get_option(name)
       elsif self.class.respond_to?(:get_option)
-- 
cgit 


From f770325884ebef493cb8ca6060a65355211125b9 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Fri, 15 Apr 2011 15:06:53 -0700
Subject: (#6978) Enforce the calling convention of option hooks.

We require that hooks take exactly three arguments; now we enforce that in the
DSL, to ensure we give good, and early, errors to users who do the wrong
thing.

Paired-With: Max Martin <max@puppetlabs.com>
---
 lib/puppet/interface/option_builder.rb | 6 ++++++
 1 file changed, 6 insertions(+)

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/option_builder.rb b/lib/puppet/interface/option_builder.rb
index ccad0850d..d4e59a4df 100644
--- a/lib/puppet/interface/option_builder.rb
+++ b/lib/puppet/interface/option_builder.rb
@@ -31,6 +31,9 @@ class Puppet::Interface::OptionBuilder
     if @option.before_action
       raise ArgumentError, "#{@option} already has a before_action set"
     end
+    unless block.arity == 3 then
+      raise ArgumentError, "before_action takes three arguments, action, args, and options"
+    end
     @option.before_action = block
   end
 
@@ -39,6 +42,9 @@ class Puppet::Interface::OptionBuilder
     if @option.after_action
       raise ArgumentError, "#{@option} already has a after_action set"
     end
+    unless block.arity == 3 then
+      raise ArgumentError, "after_action takes three arguments, action, args, and options"
+    end
     @option.after_action = block
   end
 end
-- 
cgit 


From 9264526a7cd45c9ff5767bc6c85585eb19f01f63 Mon Sep 17 00:00:00 2001
From: Pieter van de Bruggen <pieter@puppetlabs.com>
Date: Fri, 15 Apr 2011 15:31:19 -0700
Subject: (#7115) Enable default actions.

This also enables the 'help' action on the 'help'
face to serve as a default action.

Reviewed-By: Daniel Pittman
Reviewed-By: Nick Lewis
---
 lib/puppet/interface/action.rb         | 2 +-
 lib/puppet/interface/action_builder.rb | 4 ++++
 lib/puppet/interface/action_manager.rb | 9 +++++++++
 3 files changed, 14 insertions(+), 1 deletion(-)

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index db338e39e..3c18c2aaf 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -23,7 +23,7 @@ class Puppet::Interface::Action
   attr_reader :name
   def to_s() "#{@face}##{@name}" end
 
-  attr_accessor :summary
+  attr_accessor :default, :summary
 
   # 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_builder.rb b/lib/puppet/interface/action_builder.rb
index 34bb3fa44..639d8fc7f 100644
--- a/lib/puppet/interface/action_builder.rb
+++ b/lib/puppet/interface/action_builder.rb
@@ -29,6 +29,10 @@ class Puppet::Interface::ActionBuilder
     @action.add_option(option)
   end
 
+  def default
+    @action.default = true
+  end
+
   def summary(text)
     @action.summary = text
   end
diff --git a/lib/puppet/interface/action_manager.rb b/lib/puppet/interface/action_manager.rb
index d75697afa..440be2d1b 100644
--- a/lib/puppet/interface/action_manager.rb
+++ b/lib/puppet/interface/action_manager.rb
@@ -5,8 +5,13 @@ module Puppet::Interface::ActionManager
   # the code to do so.
   def action(name, &block)
     @actions ||= {}
+    @default_action ||= nil
     raise "Action #{name} already defined for #{self}" if action?(name)
     action = Puppet::Interface::ActionBuilder.build(self, name, &block)
+    if action.default
+      raise "Actions #{@default_action.name} and #{name} cannot both be default" if @default_action
+      @default_action = action
+    end
     @actions[action.name] = action
   end
 
@@ -50,6 +55,10 @@ module Puppet::Interface::ActionManager
     return result
   end
 
+  def get_default_action
+    @default_action
+  end
+
   def action?(name)
     actions.include?(name.to_sym)
   end
-- 
cgit 


From 9d2ec219bbd77bfca48a72b52fe5d0d3fcc0dcf7 Mon Sep 17 00:00:00 2001
From: Pieter van de Bruggen <pieter@puppetlabs.com>
Date: Sun, 17 Apr 2011 22:23:44 -0700
Subject: (#7013) Add support for required options.

This adds another hook into the generated wrapper, which invokes a method to
validate arguments.  This is used to raise an exception when required options
have not been passed to the method.

Reviewed-By: Daniel Pittman <daniel@puppetlabs.com>
---
 lib/puppet/interface/action.rb         | 16 +++++++++++++---
 lib/puppet/interface/option.rb         |  6 ++++--
 lib/puppet/interface/option_builder.rb |  6 +++++-
 3 files changed, 22 insertions(+), 6 deletions(-)

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index b94298963..412e39449 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -83,10 +83,10 @@ class Puppet::Interface::Action
     # 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
+    file    = __FILE__ + "+eval"
+    line    = __LINE__ + 1
     wrapper = <<WRAPPER
-def #{@name}(*args, &block)
+def #{@name}(*args)
   if args.last.is_a? Hash then
     options = args.last
   else
@@ -94,6 +94,7 @@ def #{@name}(*args, &block)
   end
 
   action = get_action(#{name.inspect})
+  action.validate_args(args)
   __invoke_decorations(:before, action, args, options)
   rval = self.__send__(#{internal_name.inspect}, *args)
   __invoke_decorations(:after, action, args, options)
@@ -142,6 +143,15 @@ WRAPPER
     option
   end
 
+  def validate_args(args)
+    required = options.map do |name|
+      get_option(name)
+    end.select(&:required?).collect(&:name) - args.last.keys
+
+    return if required.empty?
+    raise ArgumentError, "missing required options (#{required.join(', ')})"
+  end
+
   ########################################################################
   # Support code for action decoration; see puppet/interface.rb for the gory
   # details of why this is hidden away behind private. --daniel 2011-04-15
diff --git a/lib/puppet/interface/option.rb b/lib/puppet/interface/option.rb
index 2abcd4033..c04c2bf67 100644
--- a/lib/puppet/interface/option.rb
+++ b/lib/puppet/interface/option.rb
@@ -74,10 +74,12 @@ class Puppet::Interface::Option
   def optional_argument?
     !!@optional_argument
   end
-
+  def required?
+    !!@required
+  end
 
   attr_reader   :parent, :name, :aliases, :optparse
-  attr_accessor :desc
+  attr_accessor :required, :desc
 
   attr_accessor :before_action
   def before_action=(proc)
diff --git a/lib/puppet/interface/option_builder.rb b/lib/puppet/interface/option_builder.rb
index d4e59a4df..7c2ab89de 100644
--- a/lib/puppet/interface/option_builder.rb
+++ b/lib/puppet/interface/option_builder.rb
@@ -11,7 +11,7 @@ class Puppet::Interface::OptionBuilder
   def initialize(face, *declaration, &block)
     @face   = face
     @option = Puppet::Interface::Option.new(face, *declaration)
-    block and instance_eval(&block)
+    instance_eval(&block) if block_given?
     @option
   end
 
@@ -47,4 +47,8 @@ class Puppet::Interface::OptionBuilder
     end
     @option.after_action = block
   end
+
+  def required(value = true)
+    @option.required = value
+  end
 end
-- 
cgit 


From 977684ee83627194c78c81e30c001f3d9062f6f3 Mon Sep 17 00:00:00 2001
From: Pieter van de Bruggen <pieter@puppetlabs.com>
Date: Mon, 18 Apr 2011 11:10:00 -0700
Subject: (Maint) Fixing an order-dependent failure.

Running spec/unit/option_spec.rb before requiring puppet/interface
caused a circular require, and a failure.

Paired-With: Max Martin
---
 lib/puppet/interface/option.rb | 2 --
 1 file changed, 2 deletions(-)

(limited to 'lib/puppet/interface')

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


From 13e64fe9d72a0207ff65a35242d855cd3c55dd43 Mon Sep 17 00:00:00 2001
From: Max Martin <max@puppetlabs.com>
Date: Mon, 18 Apr 2011 11:06:39 -0700
Subject: (#7131) Remove support for optional arguments to options

As per the design decision documented in #7131, optional arguments to
options will no longer be supported. This patch causes such optional
arguments to raise an error, and tests for this behavior. Also cleaned
up some confusing use of the term "subject" in specs.

Paired-with: Daniel Pittman
---
 lib/puppet/interface/option.rb | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/option.rb b/lib/puppet/interface/option.rb
index c04c2bf67..5c75ffdd8 100644
--- a/lib/puppet/interface/option.rb
+++ b/lib/puppet/interface/option.rb
@@ -45,8 +45,9 @@ class Puppet::Interface::Option
 
     # 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
+    @optional_argument = @optparse.any? { |o| o=~/[ =]\[/ }
+    @optional_argument and raise ArgumentError, "Options with optional arguments are not supported"
+    if @optional_argument and not @optparse.all? { |o| o=~/[ =]\[/ } then
       raise ArgumentError, "Option #{@name} is inconsistent about the argument being optional"
     end
   end
-- 
cgit 


From 32c667c79bc0d5151580ea79719f28739945bfb1 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Sun, 17 Apr 2011 18:51:09 -0700
Subject: (#7132) Reject 'summary' text with newlines embedded.

Our summary documentation is used to provide single-line context to faces,
actions, and other items.  To support this we hard-fail if someone tries to
use the summary to embed the long documentation, and point them to the right
place to add the extended text.

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

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index b94298963..d2d4facaa 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -24,8 +24,21 @@ class Puppet::Interface::Action
   attr_reader :name
   def to_s() "#{@face}##{@name}" end
 
-  attr_accessor :default, :summary
+  attr_accessor :default
 
+  ########################################################################
+  # Documentation stuff, whee!
+  attr_accessor :summary
+  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
   # capabilities.   Heck, our initial plan was to have the faces delegate to
-- 
cgit 


From 266f937ed7d859fb2aee94c76e1a20539357c1eb Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Sun, 17 Apr 2011 19:20:49 -0700
Subject: (#6962) Add 'description' to faces and action.

This adds the 'description' method to the faces and actions, as well as
structured testing to ensure that the DSL works as expected.

Reviewed-By: Max Martin <max@puppetlabs.com>
---
 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 d2d4facaa..860ce4058 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -28,7 +28,7 @@ class Puppet::Interface::Action
 
   ########################################################################
   # Documentation stuff, whee!
-  attr_accessor :summary
+  attr_accessor :summary, :description
   def summary=(value)
     value = value.to_s
     value =~ /\n/ and
-- 
cgit 


From 22355dc01a7cab82427b43987f2bf542c281305c Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Mon, 18 Apr 2011 15:25:19 -0700
Subject: maint: test the 'help' face has the default action 'help'

We had a pending test for this, but forgot to write it way back when we were
implementing the feature.  Add it now.

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

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index 412e39449..efe7b1f50 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -24,7 +24,12 @@ class Puppet::Interface::Action
   attr_reader :name
   def to_s() "#{@face}##{@name}" end
 
-  attr_accessor :default, :summary
+  attr_accessor :default
+  def default?
+    !!@default
+  end
+
+  attr_accessor :summary
 
   # Initially, this was defined to allow the @action.invoke pattern, which is
   # a very natural way to invoke behaviour given our introspection
-- 
cgit 


From 86801b580101315706b1b02a00a36840eabd75cd Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Mon, 18 Apr 2011 13:29:47 -0700
Subject: (#7013) Support 'when_rendering' and 'render_as' in actions.

These define the API used by folks writing actions that supports their
rendering hooks.  'when_rendering' defines a helper method on the interface,
which runs the users code in their expected context.

'render_as' just sets the default rendering format; by default this is
:for_humans.

Reviewed-By: Max Martin <max@puppetlabs.com>
---
 lib/puppet/interface/action.rb         | 57 ++++++++++++++++++++++++++++++++--
 lib/puppet/interface/action_builder.rb | 44 +++++++++++++++++++++++---
 lib/puppet/interface/option.rb         |  4 +--
 lib/puppet/interface/option_builder.rb |  2 +-
 4 files changed, 96 insertions(+), 11 deletions(-)

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index efe7b1f50..bdd42b197 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -10,6 +10,8 @@ class Puppet::Interface::Action
     attrs.each do |k, v| send("#{k}=", v) end
 
     @options = {}
+    @when_rendering = {}
+    @render_as = :for_humans
   end
 
   # This is not nice, but it is the easiest way to make us behave like the
@@ -21,9 +23,9 @@ class Puppet::Interface::Action
     return bound_version
   end
 
-  attr_reader :name
   def to_s() "#{@face}##{@name}" end
 
+  attr_reader   :name
   attr_accessor :default
   def default?
     !!@default
@@ -31,6 +33,55 @@ class Puppet::Interface::Action
 
   attr_accessor :summary
 
+
+  ########################################################################
+  # Support for rendering formats and all.
+  def when_rendering(type)
+    unless type.is_a? Symbol
+      raise ArgumentError, "The rendering format must be a symbol, not #{type.class.name}"
+    end
+    @when_rendering[type]
+  end
+  def set_rendering_method_for(type, proc)
+    unless proc.is_a? Proc
+      msg = "The second argument to set_rendering_method_for must be a Proc"
+      msg += ", not #{proc.class.name}" unless proc.nil?
+      raise ArgumentError, msg
+    end
+    if proc.arity != 1 then
+      msg = "when_rendering methods take one argument, the result, not "
+      if proc.arity < 0 then
+        msg += "a variable number"
+      else
+        msg += proc.arity.to_s
+      end
+      raise ArgumentError, msg
+    end
+    unless type.is_a? Symbol
+      raise ArgumentError, "The rendering format must be a symbol, not #{type.class.name}"
+    end
+    if @when_rendering.has_key? type then
+      raise ArgumentError, "You can't define a rendering method for #{type} twice"
+    end
+    # Now, the ugly bit.  We add the method to our interface object, and
+    # retrieve it, to rotate through the dance of getting a suitable method
+    # object out of the whole process. --daniel 2011-04-18
+    @when_rendering[type] =
+      @face.__send__( :__add_method, __render_method_name_for(type), proc)
+  end
+
+  def __render_method_name_for(type)
+    :"#{name}_when_rendering_#{type}"
+  end
+  private :__render_method_name_for
+
+
+  attr_accessor :render_as
+  def render_as=(value)
+    @render_as = value.to_sym
+  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 faces delegate to
@@ -161,7 +212,7 @@ WRAPPER
   # Support code for action decoration; see puppet/interface.rb for the gory
   # details of why this is hidden away behind private. --daniel 2011-04-15
   private
-  def __decorate(type, name, proc)
-    @face.__send__ :__decorate, type, name, proc
+  def __add_method(name, proc)
+    @face.__send__ :__add_method, name, proc
   end
 end
diff --git a/lib/puppet/interface/action_builder.rb b/lib/puppet/interface/action_builder.rb
index 639d8fc7f..2ffa38709 100644
--- a/lib/puppet/interface/action_builder.rb
+++ b/lib/puppet/interface/action_builder.rb
@@ -20,20 +20,54 @@ class Puppet::Interface::ActionBuilder
   # method on the face would defer to it, but we can't get scope correct, so
   # we stick with this. --daniel 2011-03-24
   def when_invoked(&block)
-    raise "when_invoked on an ActionBuilder with no corresponding Action" unless @action
     @action.when_invoked = block
   end
 
+  def when_rendering(type = nil, &block)
+    if type.nil? then           # the default error message sucks --daniel 2011-04-18
+      raise ArgumentError, 'You must give a rendering format to when_rendering'
+    end
+    if block.nil? then
+      raise ArgumentError, 'You must give a block to when_rendering'
+    end
+    @action.set_rendering_method_for(type, block)
+  end
+
   def option(*declaration, &block)
     option = Puppet::Interface::OptionBuilder.build(@action, *declaration, &block)
     @action.add_option(option)
   end
 
-  def default
-    @action.default = true
+  def default(value = true)
+    @action.default = !!value
+  end
+
+  def render_as(value = nil)
+    value.nil? and raise ArgumentError, "You must give a rendering format to render_as"
+
+    formats = Puppet::Network::FormatHandler.formats << :for_humans
+    unless formats.include? value
+      raise ArgumentError, "#{value.inspect} is not a valid rendering format: #{formats.sort.join(", ")}"
+    end
+
+    @action.render_as = value
   end
 
-  def summary(text)
-    @action.summary = text
+  # Metaprogram the simple DSL from the target class.
+  Puppet::Interface::Action.instance_methods.grep(/=$/).each do |setter|
+    next if setter =~ /^=/
+    dsl = setter.sub(/=$/, '')
+
+    unless private_instance_methods.include? dsl
+      # 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
+    end
   end
 end
diff --git a/lib/puppet/interface/option.rb b/lib/puppet/interface/option.rb
index 1971926d8..f4c56cb2c 100644
--- a/lib/puppet/interface/option.rb
+++ b/lib/puppet/interface/option.rb
@@ -84,14 +84,14 @@ class Puppet::Interface::Option
   def before_action=(proc)
     proc.is_a? Proc or raise ArgumentError, "before action hook for #{self} is a #{proc.class.name.inspect}, not a proc"
     @before_action =
-      @parent.__send__(:__decorate, :before, __decoration_name(:before), proc)
+      @parent.__send__(:__add_method, __decoration_name(:before), proc)
   end
 
   attr_accessor :after_action
   def after_action=(proc)
     proc.is_a? Proc or raise ArgumentError, "after action hook for #{self} is a #{proc.class.name.inspect}, not a proc"
     @after_action =
-      @parent.__send__(:__decorate, :after, __decoration_name(:after), proc)
+      @parent.__send__(:__add_method, __decoration_name(:after), proc)
   end
 
   def __decoration_name(type)
diff --git a/lib/puppet/interface/option_builder.rb b/lib/puppet/interface/option_builder.rb
index 7c2ab89de..8f358c222 100644
--- a/lib/puppet/interface/option_builder.rb
+++ b/lib/puppet/interface/option_builder.rb
@@ -20,7 +20,7 @@ class Puppet::Interface::OptionBuilder
     next if setter =~ /^=/
     dsl = setter.sub(/=$/, '')
 
-    unless self.class.methods.include?(dsl)
+    unless private_instance_methods.include? dsl
       define_method(dsl) do |value| @option.send(setter, value) end
     end
   end
-- 
cgit 


From 5258e06a39e11c2cc1951af9235605a29a155c36 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Mon, 18 Apr 2011 14:41:01 -0700
Subject: (#7013) Return bound methods for when_rendering hooks.

We can return a method bound to the current face instance when we access the
'when_rendering' hook, which allows us to directly call them.  Make that
change, and add appropriate testing.

Reviewed-By: Max Martin <max@puppetlabs.com>
---
 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 bdd42b197..53c40bc51 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -40,7 +40,7 @@ class Puppet::Interface::Action
     unless type.is_a? Symbol
       raise ArgumentError, "The rendering format must be a symbol, not #{type.class.name}"
     end
-    @when_rendering[type]
+    @when_rendering[type].bind(@face)
   end
   def set_rendering_method_for(type, proc)
     unless proc.is_a? Proc
-- 
cgit 


From 5938452dccc8c925bc6275a62ae96f50916cc239 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Mon, 18 Apr 2011 15:23:03 -0700
Subject: (#7013) Strip out old face-wide rendering defaults.

Now we want to support action-based rendering, it is super-hard to define the
semantics around defaulting where things are unspecified: the execution
context (CLI, HTTP, etc) vs the face, vs the action all have different
semantics.

Without solving the problem of how we express all that context and those
semantics down in the action, especially one written by a third party, this
just becomes a box of counter-intuitive and annoying semantics and edge-cases.

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

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index 53c40bc51..8bacc216d 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -9,9 +9,9 @@ class Puppet::Interface::Action
     @name    = name.to_sym
     attrs.each do |k, v| send("#{k}=", v) end
 
-    @options = {}
+    @options        = {}
     @when_rendering = {}
-    @render_as = :for_humans
+    @render_as      = :for_humans
   end
 
   # This is not nice, but it is the easiest way to make us behave like the
-- 
cgit 


From 12098f2d54e8e00a687f42837deeef65c7759389 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Mon, 18 Apr 2011 17:21:28 -0700
Subject: (#7013) Handle rendering modes out in the application layer.

We no longer establish the rendering mode in the actions; they just default to
"nothing", and let that flow on out to the application layer.  That lets the
facade we put before the face determine the default behaviour.

This is mostly a no-op down in the CLI side, but it makes it much easier to
integrate into MCollective, HTTP-API, and for other non-CLI users of Faces.

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

(limited to 'lib/puppet/interface')

diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index 8bacc216d..d9ba4eb60 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -11,7 +11,6 @@ class Puppet::Interface::Action
 
     @options        = {}
     @when_rendering = {}
-    @render_as      = :for_humans
   end
 
   # This is not nice, but it is the easiest way to make us behave like the
-- 
cgit 


From 0fed94fbbad45388c1f9d2451d946681d80643f8 Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Mon, 18 Apr 2011 17:29:21 -0700
Subject: (#7013) Wire up rendering hooks on the CLI.

We now find, and call, the appropriate rendering hooks on actions during the
rendering phase.  This allows the user to intercept and replace the result
object that passes through the rest of the rendering system on the fly.

Example usage:

  action :foo do
    when_rendering :pson do |result|
      { :whatever => result[a],
        :foobar   => result[b],
      }
    end
  end

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 d9ba4eb60..b43895067 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -39,7 +39,8 @@ class Puppet::Interface::Action
     unless type.is_a? Symbol
       raise ArgumentError, "The rendering format must be a symbol, not #{type.class.name}"
     end
-    @when_rendering[type].bind(@face)
+    return unless @when_rendering.has_key? type
+    return @when_rendering[type].bind(@face)
   end
   def set_rendering_method_for(type, proc)
     unless proc.is_a? Proc
-- 
cgit