[webkit-reviews] review cancelled: [Bug 9234] Implement $&, $' and $` replacement codes in String.prototype.replace : [Attachment 8665] Patch without test case and change log

[bugzilla-request-daemon at opendarwin.org]

Geoffrey Garen <ggaren at apple.com> has cancelled opendarwin.org at mitzpettel.com's
request for review:
Bug 9234: Implement $&, $' and $` replacement codes in String.prototype.replace
http://bugzilla.opendarwin.org/show_bug.cgi?id=9234

Attachment 8665: Patch without test case and change log
http://bugzilla.opendarwin.org/attachment.cgi?id=8665&action=edit

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
for (int i = 0; (i = substitutedReplacement.find(UString("$"), i)) != -1; i++)
{

This line confused the dickens out of me, because 'i = 0' has no meaning, and
'i++' has meaning only as a side-effect in the test that follows it. I know
it's not something you changed, but consider one of these (maybe my perception
is skewed):

int i = -1;
while ((i = substitutedReplacement.find(UString("$"), i + 1)) != -1)

OR

for (int i = substitutedReplacement.find(UString("$")); i != -1; i =
substitutedReplacement.find(UString("$"), i + 1))

This comment would change, too:
      i += backrefLength - 1; // -1 offsets i++ (--> 'i + 1')


Anyway, looks good. We discussed how to write a layout test, so I'm going to
clear the review bit for now.