Merge branch 'bug/2.7.x/6752-allow-action-specific-render-methods'

Fix the conflicts over changes in my previous commit.

8 files changed, 152 insertions, 34 deletions

diff --git a/lib/puppet/application/face_base.rb b/lib/puppet/application/face_base.rb
index df828b5a2..9da48af55 100644
--- a/lib/puppet/application/face_base.rb
+++ b/lib/puppet/application/face_base.rb
@@ -1,6 +1,7 @@
 require 'puppet/application'
 require 'puppet/face'
 require 'optparse'
+require 'pp'
 
 class Puppet::Application::FaceBase < Puppet::Application
   should_parse_config
@@ -14,8 +15,8 @@ class Puppet::Application::FaceBase < Puppet::Application
     Puppet::Util::Log.level = :info
   end
 
-  option("--format FORMAT") do |arg|
-    @format = arg.to_sym
+  option("--render-as FORMAT") do |arg|
+    @render_as = arg.to_sym
   end
 
   option("--mode RUNMODE", "-r") do |arg|
@@ -25,7 +26,7 @@ class Puppet::Application::FaceBase < Puppet::Application
   end
 
 
-  attr_accessor :face, :action, :type, :arguments, :format
+  attr_accessor :face, :action, :type, :arguments, :render_as
   attr_writer :exit_code
 
   # This allows you to set the exit code if you don't want to just exit
@@ -34,16 +35,49 @@ class Puppet::Application::FaceBase < Puppet::Application
     @exit_code || 0
   end
 
-  # Override this if you need custom rendering.
   def render(result)
-    render_method = Puppet::Network::FormatHandler.format(format).render_method
-    if render_method == "to_pson"
-      jj result
+    format = render_as || action.render_as || :for_humans
+
+    # Invoke the rendering hook supplied by the user, if appropriate.
+    if hook = action.when_rendering(format) then
+      result = hook.call(result)
+    end
+
+    if format == :for_humans then
+      render_for_humans(result)
     else
-      result.send(render_method)
+      render_method = Puppet::Network::FormatHandler.format(format).render_method
+      if render_method == "to_pson"
+        PSON::pretty_generate(result, :allow_nan => true, :max_nesting => false)
+      else
+        result.send(render_method)
+      end
     end
   end
 
+  def render_for_humans(result)
+    # String to String
+    return result if result.is_a? String
+    return result if result.is_a? Numeric
+
+    # Simple hash to table
+    if result.is_a? Hash and result.keys.all? { |x| x.is_a? String or x.is_a? Numeric }
+      output = ''
+      column_a = result.map do |k,v| k.to_s.length end.max + 2
+      column_b = 79 - column_a
+      result.sort_by { |k,v| k.to_s } .each do |key, value|
+        output << key.to_s.ljust(column_a)
+        output << PP.pp(value, '', column_b).
+          chomp.gsub(/\n */) { |x| x + (' ' * column_a) }
+        output << "\n"
+      end
+      return output
+    end
+
+    # ...or pretty-print the inspect outcome.
+    return result.pretty_inspect
+  end
+
   def preinit
     super
     Signal.trap(:INT) do
@@ -58,9 +92,8 @@ class Puppet::Application::FaceBase < Puppet::Application
 
     # REVISIT: These should be configurable versions, through a global
     # '--version' option, but we don't implement that yet... --daniel 2011-03-29
-    @type   = self.class.name.to_s.sub(/.+:/, '').downcase.to_sym
-    @face   = Puppet::Face[@type, :current]
-    @format = @face.default_format
+    @type      = self.class.name.to_s.sub(/.+:/, '').downcase.to_sym
+    @face      = Puppet::Face[@type, :current]
 
     # Now, walk the command line and identify the action.  We skip over
     # arguments based on introspecting the action and all, and find the first
@@ -151,7 +184,7 @@ class Puppet::Application::FaceBase < Puppet::Application
     # Call the method associated with the provided action (e.g., 'find').
     if @action
       result = @face.send(@action.name, *arguments)
-      puts render(result) if result
+      puts render(result) unless result.nil?
     else
       if arguments.first.is_a? Hash
         puts "#{@face} does not have a default action"
diff --git a/lib/puppet/face/facts.rb b/lib/puppet/face/facts.rb
index 8668b2531..04eab93a5 100644
--- a/lib/puppet/face/facts.rb
+++ b/lib/puppet/face/facts.rb
@@ -2,10 +2,10 @@ require 'puppet/face/indirector'
 require 'puppet/node/facts'
 
 Puppet::Face::Indirector.define(:facts, '0.0.1') do
-  set_default_format :yaml
-
   # Upload our facts to the server
   action(:upload) do
+    render_as :yaml
+
     when_invoked do |options|
       Puppet::Node::Facts.indirection.terminus_class = :facter
       facts = Puppet::Node::Facts.indirection.find(Puppet[:certname])
diff --git a/lib/puppet/face/node.rb b/lib/puppet/face/node.rb
index fd1a548d6..12336df8f 100644
--- a/lib/puppet/face/node.rb
+++ b/lib/puppet/face/node.rb
@@ -1,5 +1,3 @@
 require 'puppet/face/indirector'
-
 Puppet::Face::Indirector.define(:node, '0.0.1') do
-  set_default_format :yaml
 end
diff --git a/lib/puppet/interface.rb b/lib/puppet/interface.rb
index 888e4ecad..5c8ade749 100644
--- a/lib/puppet/interface.rb
+++ b/lib/puppet/interface.rb
@@ -62,10 +62,8 @@ class Puppet::Interface
     end
   end
 
-  attr_accessor :default_format
-
   def set_default_format(format)
-    self.default_format = format.to_sym
+    Puppet.warning("set_default_format is deprecated (and ineffective); use render_as on your actions instead.")
   end
 
   ########################################################################
@@ -102,7 +100,6 @@ class Puppet::Interface
 
     @name = Puppet::Interface::FaceCollection.underscorize(name)
     @version = version
-    @default_format = :pson
 
     instance_eval(&block) if block_given?
   end
@@ -152,11 +149,11 @@ class Puppet::Interface
     end
   end
 
-  def __decorate(type, name, proc)
+  def __add_method(name, proc)
     meta_def(name, &proc)
     method(name).unbind
   end
-  def self.__decorate(type, name, proc)
+  def self.__add_method(name, proc)
     define_method(name, proc)
     instance_method(name)
   end
diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index f6273d1c2..23366b407 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -9,7 +9,8 @@ class Puppet::Interface::Action
     @name    = name.to_sym
     attrs.each do |k, v| send("#{k}=", v) end
 
-    @options = {}
+    @options        = {}
+    @when_rendering = {}
   end
 
   # This is not nice, but it is the easiest way to make us behave like the
@@ -21,10 +22,65 @@ 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
+  end
+
+  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
+    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
+      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
+
 
   ########################################################################
   # Documentation stuff, whee!
@@ -169,7 +225,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