You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Bhuvaneswaran Arumugam <bh...@collab.net> on 2006/08/05 10:44:32 UTC

[PATCH] contribulyze.py: Turn off html_spam_guard for title

Hello,

Please find attached the patch.

[[
* tools/dev/contribulyze.py
  (Contributor.big_name): Turn off html_spam_guard for title of the
  author page
  (Contributor.html_out): Fix the heading of patch table
]]

PROPOSAL: I wish to make changes to this script to generate the html
content based on the output of 'svn log --xml' and/or 'svn log -v
--xml'. Comments are welcome! Thank you!
-- 
Regards,
Bhuvaneswaran

Re: [PATCH] contribulyze.py: Turn off html_spam_guard for title

Posted by Max Bowsher <ma...@ukf.net>.
Bhuvaneswaran Arumugam wrote:
>>> [[
>>> * tools/dev/contribulyze.py
>>>   (Contributor.big_name): Turn off html_spam_guard for title of the
>>>   author page
>> A patch which removes the <span> tags in the <title> case would be ok,
>> but the entity-encoding should be preserved, even in the <title>.
> 
> Ok. Please find attached the revised patch.

I'll commit something in the spirit of this.

>>>   (Contributor.html_out): Fix the heading of patch table
>>> -      out.write('<td>%s</td>\n\n' % activity)
>>> +      if activity == 'Patch':
>>> +        out.write('<td>Patch List</td>\n\n')
>>> +      else:
>>> +        out.write('<td>%s</td>\n\n' % activity)
>> I do not see this as a fix, I see it as an irrelevant complication to
>> the code that adds no value.
> 
> IMO, the "Patch List" is more suitable than the word "Patch" since the
> table lists link to multiple revisions. Anyhow, i have not included this
> snippet in the attached patch.

The same argument could be made for "Review". Ultimately, I think it is 
simpler to just use the simple name of the activity type.

>>> PROPOSAL: I wish to make changes to this script to generate the html
>>> content based on the output of 'svn log --xml' and/or 'svn log -v
>>> --xml'. Comments are welcome! Thank you!
>> Quoting contribulyze.py:
>> #          .-------------------------------------------------.
>> #          |  "An ad hoc format deserves an ad hoc parser."  |
>> #          `-------------------------------------------------'
>>
>> I see no reason to use XML just for the sake of using XML, when we
>> already have proven working code for the non-XML format, if there are no
>> significant advantages in doing so.
> 
> Ok! The below quote in the same script explains that using 'xml' based
> input is the right thing to do, so, i have proposed this feature!
> 
> Quote from contribulyze.py:
> 
> # NOTES: You might be wondering, why not take 'svn log --xml' input?
> # Well, that would be the Right Thing to do, but in practice this was
> # a lot easier to whip up for straight 'svn log' output.  I'd have no
> # objection to it being rewritten to take XML input.

Hmm.

But, taking XML input means using an XML parsing library. Within the 
Python Standard Library, you have SAX or DOM. SAX leads to spaghetti 
code, and DOM leads to horribly verbose and somewhat inflexible code.

So, I'm highly reluctant to see anyone spend time re-doing functionality 
that is already there, without any real gain coming from it.

Max.


Re: [PATCH] contribulyze.py: Turn off html_spam_guard for title

Posted by Bhuvaneswaran Arumugam <bh...@collab.net>.
> > 
> > [[
> > * tools/dev/contribulyze.py
> >   (Contributor.big_name): Turn off html_spam_guard for title of the
> >   author page
> 
> A patch which removes the <span> tags in the <title> case would be ok,
> but the entity-encoding should be preserved, even in the <title>.

Ok. Please find attached the revised patch.

> >   (Contributor.html_out): Fix the heading of patch table
> > -      out.write('<td>%s</td>\n\n' % activity)
> > +      if activity == 'Patch':
> > +        out.write('<td>Patch List</td>\n\n')
> > +      else:
> > +        out.write('<td>%s</td>\n\n' % activity)
> 
> I do not see this as a fix, I see it as an irrelevant complication to
> the code that adds no value.

IMO, the "Patch List" is more suitable than the word "Patch" since the
table lists link to multiple revisions. Anyhow, i have not included this
snippet in the attached patch.

> > PROPOSAL: I wish to make changes to this script to generate the html
> > content based on the output of 'svn log --xml' and/or 'svn log -v
> > --xml'. Comments are welcome! Thank you!
> 
> Quoting contribulyze.py:
> #          .-------------------------------------------------.
> #          |  "An ad hoc format deserves an ad hoc parser."  |
> #          `-------------------------------------------------'
> 
> I see no reason to use XML just for the sake of using XML, when we
> already have proven working code for the non-XML format, if there are no
> significant advantages in doing so.

Ok! The below quote in the same script explains that using 'xml' based
input is the right thing to do, so, i have proposed this feature!

Quote from contribulyze.py:

# NOTES: You might be wondering, why not take 'svn log --xml' input?
# Well, that would be the Right Thing to do, but in practice this was
# a lot easier to whip up for straight 'svn log' output.  I'd have no
# objection to it being rewritten to take XML input.

Thank you!
-- 
Regards,
Bhuvaneswaran

Re: [PATCH] contribulyze.py: Turn off html_spam_guard for title

Posted by Max Bowsher <ma...@ukf.net>.
Bhuvaneswaran Arumugam wrote:
> Hello,
> 
> Please find attached the patch.
> 
> [[
> * tools/dev/contribulyze.py
>   (Contributor.big_name): Turn off html_spam_guard for title of the
>   author page

A patch which removes the <span> tags in the <title> case would be ok,
but the entity-encoding should be preserved, even in the <title>.

>   (Contributor.html_out): Fix the heading of patch table
> -      out.write('<td>%s</td>\n\n' % activity)
> +      if activity == 'Patch':
> +        out.write('<td>Patch List</td>\n\n')
> +      else:
> +        out.write('<td>%s</td>\n\n' % activity)

I do not see this as a fix, I see it as an irrelevant complication to
the code that adds no value.

> ]]
> 
> PROPOSAL: I wish to make changes to this script to generate the html
> content based on the output of 'svn log --xml' and/or 'svn log -v
> --xml'. Comments are welcome! Thank you!

Quoting contribulyze.py:
#          .-------------------------------------------------.
#          |  "An ad hoc format deserves an ad hoc parser."  |
#          `-------------------------------------------------'

I see no reason to use XML just for the sake of using XML, when we
already have proven working code for the non-XML format, if there are no
significant advantages in doing so.


Max.