Code smell: Line modifiers are preferred to one-line blocks.

* Replaced 6 occurances of (while .*?) *do$ with

  The do is unneeded in the block header form and causes problems
  with the block-to-one-line transformation.

  3 Examples:

      The code:
          while line = f.gets do
      becomes:
          while line = f.gets
      The code:
          while line = shadow.gets do
      becomes:
          while line = shadow.gets
      The code:
          while wrapper = zeros.pop do
      becomes:
          while wrapper = zeros.pop

* Replaced 19 occurances of ((if|unless) .*?) *then$ with

  The then is unneeded in the block header form and causes problems
  with the block-to-one-line transformation.

  3 Examples:

      The code:
          if f = test_files_for(failed).find { |f| failed_trace =~ Regexp.new(f) } then
      becomes:
          if f = test_files_for(failed).find { |f| failed_trace =~ Regexp.new(f) }
      The code:
          unless defined?(@spec_command) then
      becomes:
          unless defined?(@spec_command)
      The code:
          if c == ?\n then
      becomes:
          if c == ?\n

* Replaced 758 occurances of

      ((?:if|unless|while|until) .*)
          (.*)
      end

  with

  The one-line form is preferable provided:

      * The condition is not used to assign a variable
      * The body line is not already modified
      * The resulting line is not too long

  3 Examples:

      The code:
          if Puppet.features.libshadow?
              has_feature :manages_passwords
          end
      becomes:
          has_feature :manages_passwords if Puppet.features.libshadow?
      The code:
          unless (defined?(@current_pool) and @current_pool)
              @current_pool = process_zpool_data(get_pool_data)
          end
      becomes:
          @current_pool = process_zpool_data(get_pool_data) unless (defined?(@current_pool) and @current_pool)
      The code:
          if Puppet[:trace]
              puts detail.backtrace
          end
      becomes:
          puts detail.backtrace if Puppet[:trace]

1 files changed, 10 insertions, 30 deletions

diff --git a/lib/puppet/util/rdoc/generators/puppet_generator.rb b/lib/puppet/util/rdoc/generators/puppet_generator.rb
index f73b18dce..9d41a71ef 100644
--- a/lib/puppet/util/rdoc/generators/puppet_generator.rb
+++ b/lib/puppet/util/rdoc/generators/puppet_generator.rb
@@ -232,17 +232,13 @@ module Generators
             res1 = []
             collection['classes'].sort.each do |f|
                 if f.document_self
-                    unless f.context.is_module?
-                        res1 << { "href" => "../"+CGI.escapeHTML(f.path), "name" => CGI.escapeHTML(f.index_name) }
-                    end
+                    res1 << { "href" => "../"+CGI.escapeHTML(f.path), "name" => CGI.escapeHTML(f.index_name) } unless f.context.is_module?
                 end
             end
 
             res2 = []
             collection['methods'].sort.each do |f|
-                if f.document_self
-                    res2 << { "href" => "../#{f.path}", "name" => f.index_name.sub(/\(.*\)$/,'') }
-                end
+                res2 << { "href" => "../#{f.path}", "name" => f.index_name.sub(/\(.*\)$/,'') } if f.document_self
             end
 
             module_name = []
@@ -264,9 +260,7 @@ module Generators
 
             res5 = []
             collection['nodes'].sort.each do |f|
-                if f.document_self
-                    res5 << { "href" => "../"+CGI.escapeHTML(f.path), "name" => CGI.escapeHTML(f.name) }
-                end
+                res5 << { "href" => "../"+CGI.escapeHTML(f.path), "name" => CGI.escapeHTML(f.name) } if f.document_self
             end
 
             values = {
@@ -467,9 +461,7 @@ module Generators
         # which is also its url
         def http_url(full_name, prefix)
             path = full_name.dup
-            if path['<<']
-                path.gsub!(/<<\s*(\w*)/) { "from-#$1" }
-            end
+            path.gsub!(/<<\s*(\w*)/) { "from-#$1" } if path['<<']
             File.join(prefix, path.split("::").collect { |p| Digest::MD5.hexdigest(p) }) + ".html"
         end
 
@@ -581,9 +573,7 @@ module Generators
             c = @context
             c = c.parent while c and !c.diagram
 
-            if c && c.diagram
-                @values["diagram"] = diagram_reference(c.diagram)
-            end
+            @values["diagram"] = diagram_reference(c.diagram) if c && c.diagram
 
             @values["full_name"] = h_name
 
@@ -599,9 +589,7 @@ module Generators
                 end
                 lookup = "NODE(#{lookup})"
                 parent_url = AllReferences[lookup] || AllReferences[parent_class]
-                if parent_url and parent_url.document_self
-                    @values["par_url"] = aref_to(parent_url.path)
-                end
+                @values["par_url"] = aref_to(parent_url.path) if parent_url and parent_url.document_self
             end
 
             files = []
@@ -612,9 +600,7 @@ module Generators
                 res["full_path"]     = full_path
                 res["full_path_url"] = aref_to(f.viewer.path) if f.document_self
 
-                if @options.webcvs
-                    res["cvsurl"] = cvs_url( @options.webcvs, full_path )
-                end
+                res["cvsurl"] = cvs_url( @options.webcvs, full_path ) if @options.webcvs
 
                 files << res
             end
@@ -723,9 +709,7 @@ module Generators
         # which is also its url
         def http_url(full_name, prefix)
             path = full_name.dup
-            if path['<<']
-                path.gsub!(/<<\s*(\w*)/) { "from-#$1" }
-            end
+            path.gsub!(/<<\s*(\w*)/) { "from-#$1" } if path['<<']
             File.join(prefix, path.split("::")) + ".html"
         end
 
@@ -802,9 +786,7 @@ module Generators
                 res["full_path"]     = full_path
                 res["full_path_url"] = aref_to(f.viewer.path) if f.document_self
 
-                if @options.webcvs
-                    res["cvsurl"] = cvs_url( @options.webcvs, full_path )
-                end
+                res["cvsurl"] = cvs_url( @options.webcvs, full_path ) if @options.webcvs
 
                 files << res
             end
@@ -895,9 +877,7 @@ module Generators
 
         def find_symbol(symbol, method=nil)
             res = @context.parent.find_symbol(symbol, method)
-            if res
-                res = res.viewer
-            end
+            res = res.viewer if res
             res
         end