On 23/07/09 19:38, Brice Figureau wrote: > On 22/07/09 23:15, Luke Kanies wrote: >> On Jul 22, 2009, at 10:31 AM, Brice Figureau wrote: >> >>> On 20/07/09 8:37, Luke Kanies wrote: >>>> On Jul 18, 2009, at 4:13 AM, Brice Figureau wrote: >>>> >>>>> Puppetdoc wasn't correctly transforming AST::Boolean to string, >>>>> producing a RDoc error which needs a string. >>>>> >>>>> Signed-off-by: Brice Figureau <[email protected]> >>>>> --- >>>>> lib/puppet/util/rdoc/parser.rb | 4 ++-- >>>>> 1 files changed, 2 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/lib/puppet/util/rdoc/parser.rb b/lib/puppet/util/rdoc/ >>>>> parser.rb >>>>> index 7954865..479bb92 100644 >>>>> --- a/lib/puppet/util/rdoc/parser.rb >>>>> +++ b/lib/puppet/util/rdoc/parser.rb >>>>> @@ -426,9 +426,9 @@ class Parser >>>>> value = value.children if value.is_a? >>>>> (Puppet::Parser::AST::ASTArray) >>>>> if value.is_a?(Array) >>>>> "['#{value.join(", ")}']" >>>>> - elsif [:true, true, "true"].include?(value) >>>>> + elsif [:true, true, "true"].include?(value.to_s) >>>>> "true" >>>>> - elsif [:false, false, "false"].include?(value) >>>>> + elsif [:false, false, "false"].include?(value.to_s) >>>>> "false" >>>> Can't we just do 'elsif value.to_s == "true"' and such? >>> Actually no, because the AST::Boolean#to_s gives the real true (ie the >>> one from TrueClass). >>> So if value = AST::Boolean.new(true), then value.to_s == true, then it >>> won't == "true", which was the root cause of the issue. >>> And we also want to cover the case when value is an AST::Name (or >>> whatever) initialized with "true". >>> >>> I'm really reluctant to change AST::Boolean#to_s to actually return a >>> string, as I'm almost sure there will be hiding side effects... >>> >>> If you have a better idea let me know (and don't even think about >>> value.to_s.to_s :-)) >> Ah, so Boolean.to_s doesn't always return a string; I see. > > Yes, it returns either true or false (the booleans, not the strings). > So now, is it +1, or do you want me to do anything else?
I just see we got #2433 which is about the same kind of bug (see the bug report for an explanation of the issue). Here's what I think to definitely solve those puppetdoc issues: * make sure AST::...#to_s produce a meaningfull string for every AST node types * use exclusively to_s in puppetdoc to produce its output I was reluctant to modify to_s in the AST code, but I now think it is OK as I don't think it is used elsewhere than in displaying the parser debug log which nobody beside me or maybe Luke is using. (Of course let me know if I'm completely wrong). Does it sound correct? If it's OK, I'll fix #2433 and #2422 in the same patch thread. -- Brice Figureau My Blog: http://www.masterzen.fr/ --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
