+1 There's still some cruft in the AST code, I think, from when I was writing the parser in the first place. I think I removed most of the 'tree' stuff, but most of the debugging hasn't really been used since I got the parser fully functional. So I'm all for this kind of work.
On Jul 24, 2009, at 12:17 PM, Brice Figureau wrote: > > 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 > > > > -- Always behave like a duck - keep calm and unruffled on the surface but paddle like the devil underneath. -- Jacob Braude --------------------------------------------------------------------- Luke Kanies | http://reductivelabs.com | http://madstop.com --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
