The following discussion is an archived debate. Please do not modify it. Subsequent comments should be made in a new section. The result of the discussion was Approved.

Sambot 11

[edit]

Operator: [[Sam Korn]] (smoddy)

Automatic or Manually Assisted: Automatic

Programming Language(s): PHP, using Pillar

Function Overview: Add or remove ((puic)) per WP:PUI.

Edit period(s): Daily

Already has a bot flag (Y/N): Y

Function Details:

Code here.

[[Sam Korn]] (smoddy) 17:24, 25 April 2009 (UTC)[reply]

Discussion

[edit]

Approved for trial (3 days). Please provide a link to the relevant contributions and/or diffs when the trial is complete. (I hope it's not inappropriate for me be the one to approve this for trial.) – Quadell (talk) 01:53, 27 April 2009 (UTC)[reply]


Since we just talked about this at WT:TW/BUG#285, from a glance at the regular expressions in the code I seem to be seeing some potential problems: 
  • $regex = '/\[\[' . preg_replace ('/^File/i','(?:(?:File)|(?:Image))',str_replace(':','\:',$imagename)) . '.*/is';
    The filename needs to be properly escaped, otherwise you run into problems with File:.* and the like. Also, it doesn't accept any whitespace after the brackets, or before and after the namespace-filename seperator. Crazy stuff like [[ File  : Example.png |20px]] works: .
  • '/[\|]?\{\{puf|ic/i' is wrong, that will match anything with "ic". Should be something like '/(\|\s*)?\{\{\s*pu[fi]c/i'
  • $regex = '/\s*(?:(?:thumb)|(?:frame)|(?:border)|(?:right)|(?:left)|(?:center)|(?:none)|(?:link=)|(?:\d+px)|(?:\d+x\d+px))/i';
    should accept whitespace between the dimension and the unit, and it ignores the infamous pxpx (I sincerly hope that it's never hardcoded anywhere). Again, crazy stuff like [[File:Example.png | 20x20 pxpx ]] works: .
  • $bit .= ' ((puic|' . preg_replace('/\s*(?:(?:File)|(?:Image)):/i','',$imagename) . '|' . $puiname . '))';
    will break at File:Image:.png.
  • function kept($title,$imagename) {
[...]
    try {
[...]
    } catch (PillarException $e) {
        kept($title,$imagename);
    }
}
    looks like a stack overflow exception waiting to happen to me, if the database is locked or something, but in such a case you probably can't do anything but abort anyway. :)
Most of those won't occur often, but ...
Amalthea 14:04, 27 April 2009 (UTC)[reply]
All good points, all of them fixed. I don't know what I was doing with the escaping in that regex. The only change I haven't made is to incorporate pxpx, as it would be matched by the current regex anyway (there's no $). [[Sam Korn]] (smoddy) 17:29, 27 April 2009 (UTC)[reply]
Sure, as I said, I don't expect any pxpx outside templates.
But ... if you're not matching the entire bit, then you'll have problems with image captions like "See the topic on the right", don't you? Amalthea 17:59, 27 April 2009 (UTC)[reply]
Another good point. Fixed. [[Sam Korn]] (smoddy) 18:16, 27 April 2009 (UTC)[reply]

Trial complete. 15 edits made over the three daily runs (note that that figure will be lower than the "real" runs, as Amalthea is intending to turn off Twinkle's automatic tagging when this BRFA is approved). Listed below:

Adding ((puic))
http://en.wikipedia.org/w/index.php?title=New_Hampshire_House_of_Representatives&diff=prev&oldid=286632385
http://en.wikipedia.org/w/index.php?title=New_Hampshire_General_Court&diff=prev&oldid=286632375]
http://en.wikipedia.org/w/index.php?title=Elite_Police&diff=prev&oldid=286632164]
http://en.wikipedia.org/w/index.php?title=Elite_Police&diff=prev&oldid=286632148
http://en.wikipedia.org/w/index.php?title=Dead_letter_mail&diff=prev&oldid=286632086
http://en.wikipedia.org/w/index.php?title=Al-Khalid_tank&diff=prev&oldid=286482574
http://en.wikipedia.org/w/index.php?title=Al-Khalid_tank&diff=prev&oldid=286482516
http://en.wikipedia.org/w/index.php?title=Pakistan_Army&diff=prev&oldid=286482467
http://en.wikipedia.org/w/index.php?title=Al-Shabab_(Saudi_Arabia)&diff=prev&oldid=286481716
http://en.wikipedia.org/w/index.php?title=Edward_Low&diff=prev&oldid=286481129
http://en.wikipedia.org/w/index.php?title=Leonid_Brezhnev&diff=prev&oldid=286480356

Also three accidentally done under Sam Korn (talk · contribs):

http://en.wikipedia.org/w/index.php?title=Naypyidaw&diff=prev&oldid=286476277
http://en.wikipedia.org/w/index.php?title=Naypyidaw&diff=prev&oldid=286476966
http://en.wikipedia.org/w/index.php?title=Andrey_Kozlov&diff=prev&oldid=286477012
Removing ((puic))
http://en.wikipedia.org/w/index.php?title=Cheat_Engine&diff=prev&oldid=286482232

No problems with any of the edits.

[[Sam Korn]] (smoddy) 20:30, 29 April 2009 (UTC)[reply]

What happened here? The image is still up at PUI, so it wasn't trying to fix the superfluous pipe. Amalthea 20:39, 29 April 2009 (UTC)[reply]
Hmm, a bug that I very clearly remember fixing appears not to have been fixed ;-( Now fixed. [[Sam Korn]] (smoddy) 20:56, 29 April 2009 (UTC)[reply]

I looked over all the edits, and (Almathea's catch excepted) they all look great. Any 11th-hour concerns from anyone? – Quadell (talk) 01:06, 30 April 2009 (UTC)[reply]

Okay then!  Approved. – Quadell (talk) 21:58, 30 April 2009 (UTC)[reply]


The above discussion is preserved as an archive of the debate. Please do not modify it. Subsequent comments should be made in a new section.
Category