Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(72)

Issue 10115054: [WebUI] Fix 0-length unary term false positive in CSS presubmit. (Closed)

Created:
8 years, 8 months ago by Dan Beam
Modified:
8 years, 8 months ago
CC:
chromium-reviews, arv (Not doing code reviews), Vladislav Kaznacheev
Visibility:
Public.

Description

[WebUI] Fix 0-length unary term false positive in CSS presubmit. BUG=None (yet) TEST=./chrome/browser/resources/test_presubmit.py and no more false positives. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=133108

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+14 lines, -5 lines) Patch
M chrome/browser/resources/test_presubmit.py View 1 chunk +4 lines, -1 line 0 comments Download
M chrome/browser/resources/web_dev_style/css_checker.py View 2 chunks +10 lines, -4 lines 2 comments Download

Messages

Total messages: 6 (0 generated)
Dan Beam
8 years, 8 months ago (2012-04-19 00:17:06 UTC) #1
Dan Beam
8 years, 8 months ago (2012-04-19 04:20:41 UTC) #2
Dan Beam
ping
8 years, 8 months ago (2012-04-19 23:36:58 UTC) #3
Tyler Breisacher (Chromium)
On 2012/04/19 23:36:58, Dan Beam wrote: > ping lgtm, but you should really see if ...
8 years, 8 months ago (2012-04-20 00:25:04 UTC) #4
csilv
lgtm rubber stamp
8 years, 8 months ago (2012-04-20 01:06:03 UTC) #5
Yoyo Zhou
8 years, 8 months ago (2012-04-20 01:31:40 UTC) #6
https://chromiumcodereview.appspot.com/10115054/diff/1/chrome/browser/resourc...
File chrome/browser/resources/web_dev_style/css_checker.py (right):

https://chromiumcodereview.appspot.com/10115054/diff/1/chrome/browser/resourc...
chrome/browser/resources/web_dev_style/css_checker.py:127: zeros =
(r'^.*(?:^|\D)'
It seems like for the example "state0." in the test case, it'd be better to
change this \D to \W, or maybe the whole blob to \b.

https://chromiumcodereview.appspot.com/10115054/diff/1/chrome/browser/resourc...
chrome/browser/resources/web_dev_style/css_checker.py:129:
r'(?:\D|$)(?=[^{}]+?}).*$')
The last } is literal, right? It's confusing without an escape.
Won't this regex just not match anything outside of any curly braces?

Powered by Google App Engine
This is Rietveld 408576698