Last modified: 2013-04-22 16:15:19 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T47163, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 45163 - Gerrit: Comment parser for "r123" should not interrupt in-word (e.g. urls)
Gerrit: Comment parser for "r123" should not interrupt in-word (e.g. urls)
Status: RESOLVED FIXED
Product: Wikimedia
Classification: Unclassified
Git/Gerrit (Other open bugs)
wmf-deployment
All All
: Unprioritized normal (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-02-19 18:38 UTC by Krinkle
Modified: 2013-04-22 16:15 UTC (History)
7 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Krinkle 2013-02-19 18:38:24 UTC
The following commit message (be676ae4bb3c6e8cf8bdb6f667d8fe1b17634de9):

> Commafy support for convertNumber
> As per http://www.unicode.org/reports/tr35
> 
> Change-Id: I765298959ad10ea8561abfea487328c8ddeb39f0

Renders like:

> Commafy support for convertNumber
> 
> As per r35">http://www.unicode.org/reports/tr35
> Change-Id: I765298959ad10ea8561abfea487328c8ddeb39f0

HTML:

As per <a href="http://www.unicode.org/reports/t&lt;a href=">r35</a>"&gt;http://www.unicode.org/reports/t<a href="https://www.mediawiki.org/wiki/Special:CodeReview/MediaWiki/35">r35</a>
Comment 1 Chad H. 2013-02-19 19:44:07 UTC
We had some attempts at fixing this (Gerrit change #11589), but it wasn't perfect. There was some discussion upstream about making this less zealous (can't find the link offhand).
Comment 2 Bartosz Dziewoński 2013-02-19 19:44:59 UTC
Someone just forgot a \b at the beginning and end of their regex.
Comment 3 Bartosz Dziewoński 2013-02-19 19:46:19 UTC
Also, work like this was done for Bugzilla in Ib79728e9.
Comment 4 Andre Klapper 2013-02-19 21:07:04 UTC
Re comment 3: For reasons beyond my limited regex skills, \b obviously didn't work as expected in Bugzilla, so I replaced it by (?:^|(?<=\s))   . Works now.
Comment 5 Bartosz Dziewoński 2013-02-19 21:08:59 UTC
Yeah, I should have mentioned - IIRC \b matched between an equals sign and a letter, which meant still breaking URLs. (?:^|(?<=\s)) matches either the beginning of string or after any whitespace character, thus works.
Comment 6 Krinkle 2013-02-19 21:51:11 UTC
Well, even \b would be an improvement for gerrit as right now it is also replacing things inside words (eg. ?x=foobar123)
Comment 7 Bartosz Dziewoński 2013-02-19 22:59:24 UTC
If someone can point me to where these regexes live, I'll submit a patch to fix them.
Comment 8 Krinkle 2013-02-20 00:14:19 UTC
(templates/gerrit/gerrit.config.erb in operations-puppet.git)
> 63-[commentlink "codereview"]
> 64-	match = r(\\d+)
> 65:	link = https://www.mediawiki.org/wiki/Special:CodeReview/MediaWiki/$1


https://github.com/wikimedia/operations-puppet/blob/production/templates/gerrit/gerrit.config.erb#L55
Comment 9 Bartosz Dziewoński 2013-02-20 17:46:56 UTC
I93d1a978.
Comment 10 Bartosz Dziewoński 2013-02-20 20:07:21 UTC
Merged.

Marking this as fixed; it will probably still break if someone tries to, say, link to gitweb, but that's because gerrit's "regexes" make this pretty much impossible to implement properly like it was done for Bugzilla's parser.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links