[WebUI] Make CSS presubmit play nice with grit XML.

[WebUI] Make CSS presubmit play nice with grit XML.

R=estade@chromium.org,tbreisacher@chromium.org
TEST=No presubmit errors when <if> or <include> in CSS.
BUG=118508
NOTRY=true


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=128161

[Tyler Breisacher (Chromium), 2012-03-21 20:19:55]

lgtm

http://codereview.chromium.org/9808002/diff/5001/chrome/browser/resources/PRE...
File chrome/browser/resources/PRESUBMIT.py (right):

http://codereview.chromium.org/9808002/diff/5001/chrome/browser/resources/PRE...
chrome/browser/resources/PRESUBMIT.py:35: for presubmit in would_affect_tests:
I think there might be a nicer way to express this using filter() or any() or
something like that.

http://codereview.chromium.org/9808002/diff/5001/chrome/browser/resources/web...
File chrome/browser/resources/web_dev_style/css_checker.py (right):

http://codereview.chromium.org/9808002/diff/5001/chrome/browser/resources/web...
chrome/browser/resources/web_dev_style/css_checker.py:55: rules = filter(lambda
r: r.find(': ') >= 0, semis)
This doesn't change the behavior, right? It just makes it easier to read?

[Dan Beam, 2012-03-21 20:44:50]

http://codereview.chromium.org/9808002/diff/5001/chrome/browser/resources/PRE...
File chrome/browser/resources/PRESUBMIT.py (right):

http://codereview.chromium.org/9808002/diff/5001/chrome/browser/resources/PRE...
chrome/browser/resources/PRESUBMIT.py:35: for presubmit in would_affect_tests:
On 2012/03/21 20:19:55, Tyler Breisacher wrote:
> I think there might be a nicer way to express this using filter() or any() or
> something like that.

Done.

http://codereview.chromium.org/9808002/diff/5001/chrome/browser/resources/web...
File chrome/browser/resources/web_dev_style/css_checker.py (right):

http://codereview.chromium.org/9808002/diff/5001/chrome/browser/resources/web...
chrome/browser/resources/web_dev_style/css_checker.py:55: rules = filter(lambda
r: r.find(': ') >= 0, semis)
On 2012/03/21 20:19:55, Tyler Breisacher wrote:
> This doesn't change the behavior, right? It just makes it easier to read?

Yeah, originally I was adding something else here, but removed that code. It
ended up looking nicer, so I just left it.

[Evan Stade, 2012-03-22 02:42:15]

lgtm, but I think you need a review from maruel or someone since it's python.

[Dirk Pranke, 2012-03-22 03:05:30]

lgtm with a few nits ...

http://codereview.chromium.org/9808002/diff/8002/chrome/browser/resources/PRE...
File chrome/browser/resources/PRESUBMIT.py (right):

http://codereview.chromium.org/9808002/diff/8002/chrome/browser/resources/PRE...
chrome/browser/resources/PRESUBMIT.py:35: if filter(lambda t: t in
affected_files, would_affect_tests):
nit: I'd probably do if any(f for f in affected_files if f in
would_affect_tests)

http://codereview.chromium.org/9808002/diff/8002/chrome/browser/resources/web...
File chrome/browser/resources/web_dev_style/css_checker.py (right):

http://codereview.chromium.org/9808002/diff/8002/chrome/browser/resources/web...
chrome/browser/resources/web_dev_style/css_checker.py:54: semis = map(lambda t:
t.strip(), rule.group(1).split(';'))[0:-1]
isn't list[0:-1] == list ? can you just remove the [0:-1] ?

http://codereview.chromium.org/9808002/diff/8002/chrome/browser/resources/web...
chrome/browser/resources/web_dev_style/css_checker.py:55: rules = filter(lambda
r: r.find(': ') >= 0, semis)
Nit: this can be filter(lamba r: ': ' in r, semis)

[Dan Beam, 2012-03-22 03:19:31]

https://chromiumcodereview.appspot.com/9808002/diff/8002/chrome/browser/resou...
File chrome/browser/resources/PRESUBMIT.py (right):

https://chromiumcodereview.appspot.com/9808002/diff/8002/chrome/browser/resou...
chrome/browser/resources/PRESUBMIT.py:35: if filter(lambda t: t in
affected_files, would_affect_tests):
On 2012/03/22 03:05:30, Dirk Pranke wrote:
> nit: I'd probably do if any(f for f in affected_files if f in
> would_affect_tests)

Done. (I have little to no idea what I just did though, I'll regard it as magic)

https://chromiumcodereview.appspot.com/9808002/diff/8002/chrome/browser/resou...
File chrome/browser/resources/web_dev_style/css_checker.py (right):

https://chromiumcodereview.appspot.com/9808002/diff/8002/chrome/browser/resou...
chrome/browser/resources/web_dev_style/css_checker.py:54: semis = map(lambda t:
t.strip(), rule.group(1).split(';'))[0:-1]
On 2012/03/22 03:05:30, Dirk Pranke wrote:
> isn't list[0:-1] == list ? can you just remove the [0:-1] ?

removed 0, so [:-1] like we talked about in IRC

https://chromiumcodereview.appspot.com/9808002/diff/8002/chrome/browser/resou...
chrome/browser/resources/web_dev_style/css_checker.py:55: rules = filter(lambda
r: r.find(': ') >= 0, semis)
On 2012/03/22 03:05:30, Dirk Pranke wrote:
> Nit: this can be filter(lamba r: ': ' in r, semis)

Done.

[Dirk Pranke, 2012-03-22 03:23:56]

On Wed, Mar 21, 2012 at 8:19 PM,  <dbeam@chromium.org> wrote:
>
>
https://chromiumcodereview.appspot.com/9808002/diff/8002/chrome/browser/resou...
> File chrome/browser/resources/PRESUBMIT.py (right):
>
>
https://chromiumcodereview.appspot.com/9808002/diff/8002/chrome/browser/resou...
>
> chrome/browser/resources/PRESUBMIT.py:35: if filter(lambda t: t in
> affected_files, would_affect_tests):
> On 2012/03/22 03:05:30, Dirk Pranke wrote:
>>
>> nit: I'd probably do if any(f for f in affected_files if f in
>> would_affect_tests)
>
>
> Done. (I have little to no idea what I just did though, I'll regard it
> as magic)
>

[f in affected files if f in would_affect_tests] is the same as your
prior filter(). The any() call just returns True/False instead of a
list and short-circuits early as soon as it finds one, so I think it
captures the intent of what you're doing a bit clearer and is a little
easier to read (assuming you know about any() and comprehensions, of
course).

>
https://chromiumcodereview.appspot.com/9808002/diff/8002/chrome/browser/resou...
> File chrome/browser/resources/web_dev_style/css_checker.py (right):
>
>
https://chromiumcodereview.appspot.com/9808002/diff/8002/chrome/browser/resou...
>
> chrome/browser/resources/web_dev_style/css_checker.py:54: semis =
> map(lambda t: t.strip(), rule.group(1).split(';'))[0:-1]
> On 2012/03/22 03:05:30, Dirk Pranke wrote:
>>
>> isn't list[0:-1] == list ? can you just remove the [0:-1] ?
>
>
> removed 0, so [:-1] like we talked about in IRC
>
>
https://chromiumcodereview.appspot.com/9808002/diff/8002/chrome/browser/resou...
>
> chrome/browser/resources/web_dev_style/css_checker.py:55: rules =
> filter(lambda r: r.find(': ') >= 0, semis)
> On 2012/03/22 03:05:30, Dirk Pranke wrote:
>>
>> Nit: this can be filter(lamba r: ': ' in r, semis)
>
>
> Done.
>
> https://chromiumcodereview.appspot.com/9808002/

[commit-bot: I haz the power, 2012-03-22 03:34:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dbeam@chromium.org/9808002/15001

[commit-bot: I haz the power, 2012-03-22 03:36:03]

Change committed as 128161