Please review pull request #690: (#14072) Allow faces to inline global setting docs opened by (jeffweiss)

Description:

Add display_global_option[s] to Face DSL.
Add environment, and modulepath as display_global_options for existing
module face.
Add type for typed settings.
Add display_global_options to man Face.
Add display_global_options to help Face.

  • Opened: Thu Apr 19 21:40:45 UTC 2012
  • Based on: puppetlabs:master (a52e74fba683d9bfe0a09f5361348c696285aa82)
  • Requested merge: jeffweiss:ticket/master/14072_allow_faces_to_inline_setting_docs (b4170f4dcae3205126bea5d3603ea7970a10d865)

Diff follows:

diff --git a/lib/puppet/face/help/action.erb b/lib/puppet/face/help/action.erb
index c84e46c..5897d1d 100644
--- a/lib/puppet/face/help/action.erb
+++ b/lib/puppet/face/help/action.erb
@@ -15,8 +15,37 @@ OPTIONS:
   --verbose                      - Whether to log verbosely.
   --debug                        - Whether to log debug information.
 <% unless action.options.empty?
-     optionroom = 30
-     summaryroom = 80 - 5 - optionroom
+	optionroom = 30
+	summaryroom = 80 - 5 - optionroom
+	action.display_global_options.uniq.sort.each do |name|
+		option = name
+		desc = Puppet.settings.setting(option).desc
+		type = Puppet.settings.setting(option).default
+		type ||= Puppet.settings.setting(option).type.to_s.upcase -%>
+<%= "  " + "--#{option} #{type}".ljust(optionroom) + ' - ' -%>
+<%     	if !(desc) -%>
+undocumented option
+<%     	elsif desc.length <= summaryroom -%>
+<%= desc %>
+<%
+       	else
+			words = desc.split
+			wrapped = ['']
+			i = 0
+			words.each do |word|
+				if wrapped[i].length + word.length <= summaryroom
+					wrapped[i] << word + ' '
+				else
+					i += 1
+					wrapped[i] = word + ' '
+				end
+			end -%>
+<%= wrapped.shift.strip %>
+<%			wrapped.each do |line| -%>
+<%= (' ' * (optionroom + 5) ) + line.strip %>
+<%			end
+		 end
+	  end
       action.options.sort.each do |name|
         option = action.get_option name -%>
 <%= "  " + option.optparse.join(" | ")[0,(optionroom - 1)].ljust(optionroom) + ' - ' -%>
diff --git a/lib/puppet/face/help/man.erb b/lib/puppet/face/help/man.erb
index 5a3aa2c..254c6f0 100644
--- a/lib/puppet/face/help/man.erb
+++ b/lib/puppet/face/help/man.erb
@@ -38,6 +38,16 @@ configuration options can also be generated by running puppet with
   Whether to log verbosely.
 * --debug:
   Whether to log debug information.
+<% unless face.display_global_options.empty?
+     face.display_global_options.uniq.sort.each do |name|
+		option = name
+		desc = Puppet.settings.setting(option).desc
+		type = Puppet.settings.setting(option).default
+		type ||= Puppet.settings.setting(option).type.to_s.upcase -%>
+<%= "* --#{option} #{type}" %>:
+<%= desc.gsub(/^/, '  ') || '  undocumented setting' %>
+<%   end
+   end -%>
 <% unless face.options.empty?
      face.options.sort.each do |name|
        option = face.get_option name -%>
@@ -66,9 +76,17 @@ ACTIONS
 <%   end -%>
 
 <%   unique_options = action.options - face.options
-     unless unique_options.empty? -%>
+	 unique_display_global_options = action.display_global_options - face.display_global_options
+     unless unique_options.empty? and unique_display_global_options.empty? -%>
   `OPTIONS`
-
+<%      unique_display_global_options.uniq.sort.each do |name|
+		  option = name
+		  desc = Puppet.settings.setting(option).desc
+		  type = Puppet.settings.setting(option).default
+		  type ||= Puppet.settings.setting(option).type.to_s.upcase -%>
+  <%= "<--#{option} #{type}>" %> -
+<%= desc.gsub(/^/, '  ') %>
+<%      end -%>
 <%     unique_options.sort.each do |name|
          option = action.get_option name
          text = (option.description || option.summary).chomp + "\n" -%>
diff --git a/lib/puppet/face/module.rb b/lib/puppet/face/module.rb
index 87c14a8..5239821 100644
--- a/lib/puppet/face/module.rb
+++ b/lib/puppet/face/module.rb
@@ -14,4 +14,6 @@
     a repository of user-contributed Puppet code. It can also generate empty
     modules, and prepare locally developed modules for release on the Forge.
   EOT
+  
+  display_global_options "environment", "modulepath"
 end
diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index 5f23f55..ba44c3b 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -22,6 +22,7 @@ def initialize(face, name, attrs = {})
     # @options collects the added options in the order they're declared.
     # @options_hash collects the options keyed by alias for quick lookups.
     @options        = []
+    @display_global_options = []
     @options_hash   = {}
     @when_rendering = {}
   end
@@ -246,6 +247,19 @@ def option?(name)
   def options
     @face.options + @options
   end
+  
+  def add_display_global_options(*args)
+    [args].flatten.each do |refopt|
+      raise ArgumentError, "Global option #{refopt} does not exist in Puppet.settings" unless Puppet.settings.include? refopt
+      @display_global_options << refopt
+    end
+    @display_global_options.uniq!
+    args
+  end
+  
+  def display_global_options
+    @face.display_global_options + @display_global_options
+  end
 
   def get_option(name, with_inherited_options = true)
     option = @options_hash[name.to_sym]
diff --git a/lib/puppet/interface/action_builder.rb b/lib/puppet/interface/action_builder.rb
index 4948f5f..2c60b11 100644
--- a/lib/puppet/interface/action_builder.rb
+++ b/lib/puppet/interface/action_builder.rb
@@ -34,6 +34,10 @@ def option(*declaration, &block)
   def default(value = true)
     @action.default = !!value
   end
+  
+  def display_global_options(*args)
+    @action.add_display_global_options [args]
+  end
 
   def render_as(value = nil)
     value.nil? and raise ArgumentError, "You must give a rendering format to render_as"
diff --git a/lib/puppet/interface/documentation.rb b/lib/puppet/interface/documentation.rb
index 47e478a..7d8ca74 100644
--- a/lib/puppet/interface/documentation.rb
+++ b/lib/puppet/interface/documentation.rb
@@ -85,6 +85,18 @@ def build_synopsis(face, action = "" arguments = nil)
 
           s.breakable
         end
+        
+        display_global_options.uniq.sort.each do |option|
+          wrap = %w{ [ ] }
+          s.group(0, *wrap) do
+        		desc = Puppet.settings.setting(option).desc
+        		type = Puppet.settings.setting(option).default
+        		type ||= Puppet.settings.setting(option).type.to_s.upcase
+            s.text "--#{option} #{type}"
+            s.breakable
+          end
+          s.breakable
+        end
 
         if arguments then
           s.text arguments.to_s
diff --git a/lib/puppet/interface/option_manager.rb b/lib/puppet/interface/option_manager.rb
index a1f300e..492d58c 100644
--- a/lib/puppet/interface/option_manager.rb
+++ b/lib/puppet/interface/option_manager.rb
@@ -1,6 +1,31 @@
 require 'puppet/interface/option_builder'
 
 module Puppet::Interface::OptionManager
+  
+  def display_global_options(*args)
+    @display_global_options ||= []
+    [args].flatten.each do |refopt|
+      @display_global_options << refopt if refopt
+    end
+    @display_global_options.uniq!
+    args
+  end
+  alias :display_global_option :display_global_options
+  
+  def all_display_global_options
+    walk_inheritance_tree(@display_global_options, :all_display_global_options)
+  end
+  
+  def walk_inheritance_tree(start, sym)
+    result = (start ||= [])
+    if self.is_a?(Class) and superclass.respond_to?(sym)
+      result = superclass.send(sym) + result
+    elsif self.class.respond_to?(sym)
+      result = self.class.send(sym) + result
+    end
+    return result
+  end
+  
   # Declare that this app can take a specific option, and provide
   # the code to do so.
   def option(*declaration, &block)
@@ -36,15 +61,7 @@ def add_option(option)
   end
 
   def options
-    result = (@options ||= [])
-
-    if self.is_a?(Class) and superclass.respond_to?(:options)
-      result = superclass.options + result
-    elsif self.class.respond_to?(:options)
-      result = self.class.options + result
-    end
-
-    return result
+    walk_inheritance_tree(@options, :options)
   end
 
   def get_option(name, with_inherited_options = true)
diff --git a/lib/puppet/util/settings/boolean_setting.rb b/lib/puppet/util/settings/boolean_setting.rb
index d11e4c8..53c91a2 100644
--- a/lib/puppet/util/settings/boolean_setting.rb
+++ b/lib/puppet/util/settings/boolean_setting.rb
@@ -27,4 +27,7 @@ def munge(value)
       raise ArgumentError, "Invalid value '#{value.inspect}' for #{@name}"
     end
   end
+  def type
+    :boolean
+  end
 end
diff --git a/lib/puppet/util/settings/string_setting.rb b/lib/puppet/util/settings/string_setting.rb
index 7405952..b72ac0d 100644
--- a/lib/puppet/util/settings/string_setting.rb
+++ b/lib/puppet/util/settings/string_setting.rb
@@ -19,6 +19,10 @@ def setbycli=(value)
     @settings.set_value(name, @settings[name], :cli) if value
     raise ArgumentError, "Cannot unset setbycli" unless value
   end
+  
+  def type
+    :string
+  end
 
   # get the arguments in getopt format
   def getopt_args
diff --git a/spec/unit/interface/documentation_spec.rb b/spec/unit/interface/documentation_spec.rb
index 6865b90..d9c55bd 100755
--- a/spec/unit/interface/documentation_spec.rb
+++ b/spec/unit/interface/documentation_spec.rb
@@ -6,10 +6,11 @@
 
 class Puppet::Interface::TinyDocs::Test
   include Puppet::Interface::TinyDocs
-  attr_accessor :name, :options
+  attr_accessor :name, :options, :display_global_options
   def initialize
     self.name    = "tinydoc-test"
     self.options = []
+    self.display_global_options = []
   end
 
   def get_option(name)
diff --git a/spec/unit/interface_spec.rb b/spec/unit/interface_spec.rb
index 1621b9a..f7a7ee4 100755
--- a/spec/unit/interface_spec.rb
+++ b/spec/unit/interface_spec.rb
@@ -136,6 +136,44 @@ def add_options_to(&block)
       subject.new(:with_options, '0.0.1', &block)
     end
   end
+  
+  describe "with face-level display_global_options" do
+    it "should not return any action level display_global_options" do
+      face = subject.new(:with_display_global_options, '0.0.1') do
+        display_global_options "environment"
+        action :baz do
+          when_invoked {|_| true }
+          display_global_options "modulepath"
+        end
+      end
+      face.display_global_options =~ ["environment"]
+    end
+        
+    it "should not fail when a face d_g_o duplicates an action d_g_o" do
+      expect {
+        subject.new(:action_level_display_global_options, '0.0.1') do
+          action :bar do
+            when_invoked {|_| true }
+            display_global_options "environment"
+          end
+          display_global_options "environment"
+        end
+      }.should_not raise_error
+    end
+    
+    it "should work when two actions have the same d_g_o" do
+      face = subject.new(:with_display_global_options, '0.0.1') do
+        action :foo do when_invoked {|_| true} ; display_global_options "environment" end
+        action :bar do when_invoked {|_| true} ; display_global_options "environment" end
+      end
+      face.get_action(:foo).display_global_options =~ ["environment"]
+      face.get_action(:bar).display_global_options =~ ["environment"]
+    end
+      
+  end
+  
+  describe "with inherited display_global_options" do
+  end
 
   describe "with face-level options" do
     it "should not return any action-level options" do

    

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.

Reply via email to