web_dev_style: ignore --css-vars from alphabetical ordering requirement

chrome/browser/web_dev_style/css_checker.py

Index: chrome/browser/web_dev_style/css_checker.py
diff --git a/chrome/browser/web_dev_style/css_checker.py b/chrome/browser/web_dev_style/css_checker.py
index 77f610f4db804a7f0c0e52f8400ad54333d9de5d..77c0c65dcae17a38a7b6a748c84477c0de23f102 100644
--- a/chrome/browser/web_dev_style/css_checker.py
+++ b/chrome/browser/web_dev_style/css_checker.py
@@ -33,7 +33,7 @@ class CSSChecker(object):
       s = _remove_ats(s)
       s = _remove_comments(s)
       s = _remove_template_expressions(s)
-      s = _remove_mixins(s)
+      s = _remove_valid_vars(s)
       return s
 
     def _extract_inline_style(s):
@@ -52,12 +52,6 @@ class CSSChecker(object):
     def _remove_comments(s):
       return re.sub(re.compile(r'/\*.*?\*/', re.DOTALL), '', s)
 
-    def _remove_mixins(s):
-      return re.sub(re.compile(r'--[\d\w-]+: {.*?};', re.DOTALL), '', s)

[dschuyler, 2016/09/19 21:23:28]

With this no longer being done, it looks like
something like this, "--foo: {\n a: b;\n c: d;\n};\n"
would result in, '\n c: d;\n};\n' based on the new
_remove_valid_vars().

-
-    def _remove_template_expressions(s):
-      return re.sub(re.compile(r'\$i18n(Raw)?{[^}]*}', re.DOTALL), '', s)
-
     def _remove_grit(s):
       grit_reg = re.compile(r"""
           <if[^>]+>.*?<\s*/\s*if[^>]*>|  # <if> contents </if>
@@ -66,6 +60,15 @@ class CSSChecker(object):
           re.DOTALL | re.VERBOSE)
       return re.sub(grit_reg, '', s)
 
+    def _remove_template_expressions(s):
+      return re.sub(re.compile(r'\$i18n(Raw)?{[^}]*}', re.DOTALL), '', s)
+
+    mixin_shim_reg = '[\w\d-]+_-_[\w\d-]+'
+
+    def _remove_valid_vars(s):
+      valid_var_reg = '--(?!' + mixin_shim_reg + ')[\d\w-]+\s*:\s*[^;]+?;'
+      return re.sub(re.compile(valid_var_reg, re.DOTALL), '', s)

[dschuyler, 2016/09/19 21:23:28]

About the regex changes:
- having \d and \w in the same [] is be redundant (previously mentioned).
- I think the re.DOTALL doesn't affect a regex that doesn't have any dots '.'
- If the re.DOTALL is removed, the re.compile would become redundant.
- the \s* near the end looks redundant with the [^;]+, since 'not ;' would
include \s.
- the ? near the end "[^;]+?;" (while harmless) doesn't appear to be useful
- if the ? was trying to make the "[^;]+" optional, then either of these would
work as alternatives: "([^;]+)?" or "[^;]*"
- the backslashes happen to be working out ok with \w and \s, but I think it
would be more correct to create raw (r'') strings

Does something like the following do what is needed?

   mixin_shim_reg = r'[\w-]+_-_[\w-]+'

   def _remove_valid_vars(s):
      valid_var_reg = '--(?!' + mixin_shim_reg + r')[\w-]+\s*:[^;]*;'
      return re.sub(valid_var_reg, '', s)

[Dan Beam, 2016/09/22 02:13:26]

Done.
added back a .
i kept it, but turns out you can just pass flags= as well.  changed all _remove*
methods to just use re.sub(..., flags=...) rather than compile()
Done.
Done.
no, we talked about this.  was supposed to be non-greedy.  it probably doesn't
matter.  dropped other than for .*?
Done.
kind of, but like you mentioned (somewhere else?) [^;] can only be satisfied
until the first ;.  inside of multi-rule mixins, this is problematic.

so i've essentially created 2 branches: if we find a {.*?}, remove that, else
look until ;

+
     def _rgb_from_hex(s):
       if len(s) == 3:
         r, g, b = s[0] + s[0], s[1] + s[1], s[2] + s[2]
@@ -170,7 +173,7 @@ class CSSChecker(object):
       return re.search(r'\(\s*\s*data:', line)
 
     def no_mixin_shims(line):
-      return re.search('\-\-[\w\-]+_\-_[\w\-]+\s*:', line)
+      return re.search('--' + mixin_shim_reg + '\s*:', line)
 
     def no_quotes_in_url(line):
       return re.search('url\s*\(\s*["\']', line, re.IGNORECASE)