Here is the new version of a patch I sent earlier to fix puppetdoc.
Instead of changing puppetdoc, I chose to implement proper #to_s
methods in some of the AST nodes (the ones puppetdoc knows how to parse).
I don't think it introduces any issues (ie AST nodes #to_s is used
only when the racc parser is in debug mode which happens only in specific
debugging situation), but if someone thinks there would, please speak up.

Please review,
Brice

Original Commit Msg:
AST nodes don't have a valid to_s that is producing a correct
representation of said node.
This patch adds some of the AST node to_s to produce correct
values that can be used verbatim by puppetdoc to render
the documentation.

Signed-off-by: Brice Figureau <[email protected]>
---
 lib/puppet/parser/ast/astarray.rb           |    4 +++
 lib/puppet/parser/ast/function.rb           |    5 ++++
 lib/puppet/parser/ast/leaf.rb               |   14 +++++++++++-
 lib/puppet/parser/ast/resource_reference.rb |    8 +++++++
 lib/puppet/util/rdoc/parser.rb              |   29 ++++--------------------
 spec/unit/parser/ast/astarray.rb            |    6 +++++
 spec/unit/parser/ast/function.rb            |    6 +++++
 spec/unit/parser/ast/leaf.rb                |   31 +++++++++++++++++++++++++++
 spec/unit/parser/ast/resource_reference.rb  |    6 +++++
 9 files changed, 84 insertions(+), 25 deletions(-)
 create mode 100755 spec/unit/parser/ast/leaf.rb

diff --git a/lib/puppet/parser/ast/astarray.rb 
b/lib/puppet/parser/ast/astarray.rb
index 9b59d19..dfd2bcd 100644
--- a/lib/puppet/parser/ast/astarray.rb
+++ b/lib/puppet/parser/ast/astarray.rb
@@ -47,6 +47,10 @@ class Puppet::Parser::AST
 
             return self
         end
+
+        def to_s
+            "[" + @children.collect { |c| c.to_s }.join(', ') + "]"
+        end
     end
 
     # A simple container class, containing the parameters for an object.
diff --git a/lib/puppet/parser/ast/function.rb 
b/lib/puppet/parser/ast/function.rb
index 2f768eb..4c3a5dc 100644
--- a/lib/puppet/parser/ast/function.rb
+++ b/lib/puppet/parser/ast/function.rb
@@ -51,5 +51,10 @@ class Puppet::Parser::AST
              @fname = Puppet::Parser::Functions.function(@name)
             # Lastly, check the parity
         end
+
+        def to_s
+            args = arguments.is_a?(ASTArray) ? 
arguments.to_s.gsub(/\[(.*)\]/,'\1') : arguments
+            "#{name}(#{args})"
+        end
     end
 end
diff --git a/lib/puppet/parser/ast/leaf.rb b/lib/puppet/parser/ast/leaf.rb
index dba9812..d089380 100644
--- a/lib/puppet/parser/ast/leaf.rb
+++ b/lib/puppet/parser/ast/leaf.rb
@@ -11,7 +11,7 @@ class Puppet::Parser::AST
         end
 
         def to_s
-            return @value
+            return @value.to_s unless @value.nil?
         end
     end
 
@@ -29,6 +29,10 @@ class Puppet::Parser::AST
             end
             @value
         end
+
+        def to_s
+            @value ? "true" : "false"
+        end
     end
 
     # The base string class.
@@ -38,6 +42,10 @@ class Puppet::Parser::AST
         def evaluate(scope)
             return scope.strinterp(@value, file, line)
         end
+
+        def to_s
+            "\"#...@value}\""
+        end
     end
 
     # An uninterpreted string.
@@ -45,6 +53,10 @@ class Puppet::Parser::AST
         def evaluate(scope)
             return @value
         end
+
+        def to_s
+            "\"#...@value}\""
+        end
     end
 
     # The 'default' option on case statements and selectors.
diff --git a/lib/puppet/parser/ast/resource_reference.rb 
b/lib/puppet/parser/ast/resource_reference.rb
index 6189ab8..117bc88 100644
--- a/lib/puppet/parser/ast/resource_reference.rb
+++ b/lib/puppet/parser/ast/resource_reference.rb
@@ -64,5 +64,13 @@ class Puppet::Parser::AST
             end
             return objtype
         end
+
+        def to_s
+            if title.is_a?(ASTArray)
+                "#{type.to_s.capitalize}#{title}"
+            else
+                "#{type.to_s.capitalize}[#{title}]"
+            end
+        end
     end
 end
diff --git a/lib/puppet/util/rdoc/parser.rb b/lib/puppet/util/rdoc/parser.rb
index 7954865..324b6b7 100644
--- a/lib/puppet/util/rdoc/parser.rb
+++ b/lib/puppet/util/rdoc/parser.rb
@@ -155,8 +155,8 @@ class Parser
             scan_for_vardef(container,stmt.children) if 
stmt.is_a?(Puppet::Parser::AST::ASTArray)
 
             if stmt.is_a?(Puppet::Parser::AST::VarDef)
-                Puppet.debug "rdoc: found constant: %s = %s" % 
[stmt.name.to_s, value_to_s(stmt.value)]
-                container.add_constant(Constant.new(stmt.name.to_s, 
value_to_s(stmt.value), stmt.doc))
+                Puppet.debug "rdoc: found constant: %s = %s" % 
[stmt.name.to_s, stmt.value.to_s]
+                container.add_constant(Constant.new(stmt.name.to_s, 
stmt.value.to_s, stmt.doc))
             end
         end
     end
@@ -169,20 +169,15 @@ class Parser
 
             if stmt.is_a?(Puppet::Parser::AST::Resource) and !stmt.type.nil?
                 type = stmt.type.split("::").collect { |s| s.capitalize 
}.join("::")
-                title = value_to_s(stmt.title)
+                title = stmt.title.is_a?(Puppet::Parser::AST::ASTArray) ? 
stmt.title.to_s.gsub(/\[(.*)\]/,'\1') : stmt.title.to_s
                 Puppet.debug "rdoc: found resource: %s[%s]" % [type,title]
 
                 param = []
                 stmt.params.children.each do |p|
                     res = {}
                     res["name"] = p.param
-                    if !p.value.nil?
-                        if !p.value.is_a?(Puppet::Parser::AST::ASTArray)
-                            res["value"] = "'#{p.value}'"
-                        else
-                            res["value"] = "[%s]" % p.value.children.collect { 
|v| "'#{v}'" }.join(", ")
-                        end
-                    end
+                    res["value"] = "#{p.value.to_s}" unless p.value.nil?
+
                     param << res
                 end
 
@@ -420,19 +415,5 @@ class Parser
         comment.gsub!(/^#--.*?^#\+\+/m, '')
         comment.sub!(/^#--.*/m, '')
     end
-
-    # convert an AST value to a string
-    def value_to_s(value)
-        value = value.children if value.is_a?(Puppet::Parser::AST::ASTArray)
-        if value.is_a?(Array)
-            "['#{value.join(", ")}']"
-        elsif [:true, true, "true"].include?(value)
-            "true"
-        elsif [:false, false, "false"].include?(value)
-            "false"
-        else
-            value.to_s
-        end
-    end
 end
 end
\ No newline at end of file
diff --git a/spec/unit/parser/ast/astarray.rb b/spec/unit/parser/ast/astarray.rb
index 212c3fd..1791c71 100755
--- a/spec/unit/parser/ast/astarray.rb
+++ b/spec/unit/parser/ast/astarray.rb
@@ -62,5 +62,11 @@ describe Puppet::Parser::AST::ASTArray do
         operator.evaluate(@scope).should == [[123]]
     end
 
+    it "should return a valid string with to_s" do
+        a = stub 'a', :is_a? => true, :to_s => "a"
+        b = stub 'b', :is_a? => true, :to_s => "b"
+        array = Puppet::Parser::AST::ASTArray.new :children => [a,b]
 
+        array.to_s.should == "[a, b]"
+    end
 end
diff --git a/spec/unit/parser/ast/function.rb b/spec/unit/parser/ast/function.rb
index 1542013..bb687ea 100644
--- a/spec/unit/parser/ast/function.rb
+++ b/spec/unit/parser/ast/function.rb
@@ -16,6 +16,12 @@ describe Puppet::Parser::AST::Function do
         end
     end
 
+    it "should return its representation with to_s" do
+        args = stub 'args', :is_a? => true, :to_s => "[a, b]"
+
+        Puppet::Parser::AST::Function.new(:name => "func", :arguments => 
args).to_s.should == "func(a, b)"
+    end
+
     describe "when evaluating" do
 
         it "should fail if the function doesn't exist" do
diff --git a/spec/unit/parser/ast/leaf.rb b/spec/unit/parser/ast/leaf.rb
new file mode 100755
index 0000000..5ca7bc6
--- /dev/null
+++ b/spec/unit/parser/ast/leaf.rb
@@ -0,0 +1,31 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../../spec_helper'
+
+describe Puppet::Parser::AST::Leaf do
+    describe "when converting to string" do
+        it "should transform its value to string" do
+            value = stub 'value', :is_a? => true
+            value.expects(:to_s)
+            Puppet::Parser::AST::Leaf.new( :value => value ).to_s
+        end
+    end
+end
+
+describe Puppet::Parser::AST::FlatString do
+    describe "when converting to string" do
+        it "should transform its value to a quoted string" do
+            value = stub 'value', :is_a? => true, :to_s => "ab"
+            Puppet::Parser::AST::FlatString.new( :value => value ).to_s.should 
== "\"ab\""
+        end
+    end
+end
+
+describe Puppet::Parser::AST::FlatString do
+    describe "when converting to string" do
+        it "should transform its value to a quoted string" do
+            value = stub 'value', :is_a? => true, :to_s => "ab"
+            Puppet::Parser::AST::String.new( :value => value ).to_s.should == 
"\"ab\""
+        end
+    end
+end
diff --git a/spec/unit/parser/ast/resource_reference.rb 
b/spec/unit/parser/ast/resource_reference.rb
index 3a759c5..24865e8 100755
--- a/spec/unit/parser/ast/resource_reference.rb
+++ b/spec/unit/parser/ast/resource_reference.rb
@@ -60,4 +60,10 @@ describe Puppet::Parser::AST::ResourceReference do
         ref.evaluate(@scope)
     end
 
+    it "should return a correct representation when converting to string" do
+        type = stub 'type', :is_a? => true, :to_s => "file"
+        title = stub 'title', :is_a? => true, :to_s => "[/tmp/a, /tmp/b]"
+
+        ast::ResourceReference.new( :type => type, :title => title 
).to_s.should == "File[/tmp/a, /tmp/b]"
+    end
 end
-- 
1.6.0.2


--~--~---------~--~----~------------~-------~--~----~
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