(#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.

5 files changed, 255 insertions, 184 deletions

diff --git a/lib/puppet/interface.rb b/lib/puppet/interface.rb
index adf6c991c..23d760b30 100644
--- a/lib/puppet/interface.rb
+++ b/lib/puppet/interface.rb
@@ -1,8 +1,11 @@
 require 'puppet'
 require 'puppet/util/autoload'
+require 'puppet/interface/documentation'
 require 'prettyprint'
 
 class Puppet::Interface
+  include DocSupport
+
   require 'puppet/interface/face_collection'
 
   require 'puppet/interface/action_manager'
@@ -66,173 +69,12 @@ class Puppet::Interface
     Puppet.warning("set_default_format is deprecated (and ineffective); use render_as on your actions instead.")
   end
 
+
   ########################################################################
   # Documentation.  We currently have to rewrite both getters because we share
   # the same instance between build-time and the runtime instance.  When that
   # splits out this should merge into a module that both the action and face
   # include. --daniel 2011-04-17
-  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
-      fail "REVISIT: Extract this..."
-    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
-
   def synopsis
     output = PrettyPrint.format do |s|
       s.text("puppet #{name} <action>")
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.
@@ -83,18 +115,6 @@ class Puppet::Interface::Action
 
 
   ########################################################################
-  # 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
   # capabilities.   Heck, our initial plan was to have the faces delegate to
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
diff --git a/spec/shared_behaviours/documentation_on_faces.rb b/spec/shared_behaviours/documentation_on_faces.rb
index ef2645620..dd2bd3110 100644
--- a/spec/shared_behaviours/documentation_on_faces.rb
+++ b/spec/shared_behaviours/documentation_on_faces.rb
@@ -38,6 +38,46 @@ shared_examples_for "documentation on faces" do
     end
   end
 
+  describe "#short_description" do
+    it "should return the set value if set after description" do
+      subject.description = "hello\ngoodbye"
+      subject.short_description = "whatever"
+      subject.short_description.should == "whatever"
+    end
+
+    it "should return the set value if set before description" do
+      subject.short_description = "whatever"
+      subject.description = "hello\ngoodbye"
+      subject.short_description.should == "whatever"
+    end
+
+    it "should return nothing if not set and no description" do
+      subject.short_description.should be_nil
+    end
+
+    it "should return the first paragraph of description if not set (where it is one line long)" do
+      subject.description = "hello"
+      subject.short_description.should == subject.description
+    end
+
+    it "should return the first paragraph of description if not set (where there is no paragraph break)" do
+      subject.description = "hello\ngoodbye"
+      subject.short_description.should == subject.description
+    end
+
+    it "should return the first paragraph of description if not set (where there is a paragraph break)" do
+      subject.description = "hello\ngoodbye\n\nmore\ntext\nhere\n\nfinal\nparagraph"
+      subject.short_description.should == "hello\ngoodbye"
+    end
+
+    it "should trim a very, very long first paragraph" do
+      line = "this is a very, very, very long long line full of text\n"
+      subject.description = line * 20 + "\n\nwhatever, dude."
+
+      subject.short_description.should == (line * 5).chomp
+    end
+  end
+
   describe "multiple authors" do
     authors = %w{John Paul George Ringo}
 
@@ -86,11 +126,6 @@ shared_examples_for "documentation on faces" do
       subject.license.should =~ /All Rights Reserved/
     end
 
-    it "should accept an arbitrary license string in the DSL" do
-      subject.license("foo")
-      subject.license.should == "foo"
-    end
-
     it "should accept an arbitrary license string on the object" do
       subject.license = "foo"
       subject.license.should == "foo"
@@ -173,6 +208,10 @@ shared_examples_for "documentation on faces" do
       it "should have a #{attr}" do
         subject.send(attr).should_not be_nil
       end
+
+      it "'s #{attr} should not be empty..." do
+        subject.send(attr).should_not == ''
+      end
     end
   end
 end