On Jul 23, 2009, at 11:15 AM, Brice Figureau wrote: > > 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.
As indicated by the acceptance of the patch, I'm all for this. -- Somebody has to do something, and it's just incredibly pathetic that it has to be us. --Jerry Garcia --------------------------------------------------------------------- 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 -~----------~----~----~----~------~----~------~--~---
