Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29867555: Noissue - Check for comment indicator first while parsing filter text (Closed)

Created:
Aug. 28, 2018, 7:29 a.m. by Manish Jethani
Modified:
Aug. 29, 2018, 4:32 p.m.
Reviewers:
hub, Jon Sonesen
Base URL:
https://hg.adblockplus.org/adblockpluscore/
Visibility:
Public.

Description

It makes sense to check for "!" first before doing any further processing, especially given that we're looking for "#" throughout the string.

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -5 lines) Patch
M lib/filterClasses.js View 1 chunk +10 lines, -5 lines 2 comments Download

Messages

Total messages: 5
Manish Jethani
Aug. 28, 2018, 7:29 a.m. (2018-08-28 07:29:45 UTC) #1
Manish Jethani
Patch Set 1
Aug. 28, 2018, 7:38 a.m. (2018-08-28 07:38:13 UTC) #2
Jon Sonesen
https://codereview.adblockplus.org/29867555/diff/29867556/lib/filterClasses.js File lib/filterClasses.js (right): https://codereview.adblockplus.org/29867555/diff/29867556/lib/filterClasses.js#newcode126 lib/filterClasses.js:126: if (text[0] == "!") Is there any difference performance ...
Aug. 29, 2018, 3:01 p.m. (2018-08-29 15:01:36 UTC) #3
Manish Jethani
https://codereview.adblockplus.org/29867555/diff/29867556/lib/filterClasses.js File lib/filterClasses.js (right): https://codereview.adblockplus.org/29867555/diff/29867556/lib/filterClasses.js#newcode126 lib/filterClasses.js:126: if (text[0] == "!") On 2018/08/29 15:01:36, Jon Sonesen ...
Aug. 29, 2018, 3:50 p.m. (2018-08-29 15:50:15 UTC) #4
hub
Aug. 29, 2018, 4:14 p.m. (2018-08-29 16:14:23 UTC) #5
LGTM

Powered by Google App Engine
This is Rietveld