Patches item #2424422, was opened at 2008-12-13 21:34
Message generated for change (Comment added) made by nicdumz
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=603140&aid=242442…
Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Rick Block (rick_block)
Assigned to: Nobody/Anonymous (nobody)
Summary: -content option for replace.py
Initial Comment:
I have a number of tools I run at en.wikipedia that download pages, manipulate the pages
using tools like awk, and then upload new versions of the pages. I've implemented an
option to replace.py (-content) to provide a filename containing the replacement content
for a page. The svn.diff file is attached. The same option can be used to create a new
page with content from a specified file as well.
----------------------------------------------------------------------
Comment By: NicDumZ — Nicolas Dumazet (nicdumz)
Date: 2009-04-27 18:13
Message:
Sorry for taking so long to review this.
I'm not sure that I understand what would be the usage of that script.
Could you provide a (commandline) example, and explain maybe a bit more
what would be an interesting application of it?
In fact, I dont really understand why one would need to use replace.py,
which is a rather complex script, to simple replace all the text in a page
with an arbitrary other text:
for page in gen:
page.put(text)
does the same thing. You have to add 4 or 5 lines to handle command line
arguments, of course, but using replace.py looks a bit "overkill" =)
But maybe I'm wrong here, so please include a use case :)
On the patch itself, if you still want me to include it:
* This patch mixes tab and spaces, which is a very bad practice in Python.
Please fix this
* Please make sure that replacement_text has a default None value in the
Bot constructor. Please also append it at the _end_ of the constructor
signature, and not in the middle of the arguments list, to ensure backwards
compatibility.
* Please be a bit more verbose in the documentation of the -content
option. If you think that -content will be useful to other users, you'll
have to explain them _why_ this is useful =) A commandline example, and a
real application could help here.
* + new_text = new_text.replace(u'\n',u'\r\n')
+ if ( new_text == original_text ) or ( new_text ==
original_text + '\r\n'):
What is this ? This doesn't look quite good (two text comparisons instead
of one?), it's not documented, and it doesn't look related to the patch
topic. (does it?)
* While updating your patch, please also update your patch against HEAD
Thanks :)
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=603140&aid=242442…