Skip to content

Update /r2/r2/lib/cssfilter.py: minor cleanup and support for CSS3 transforms #368

Closed
wants to merge 1 commit into from

3 participants

@Dakta
Dakta commented Mar 12, 2012

Done:
1. cleanup on some CSS property regexes (replaced \s+ with {w});
2. add CSS3 transform and transform-origin properties;
3. add associated translation-value, translation-function, and angle macros.

@Dakta Dakta Update /r2/r2/lib/cssfilter.py: cleanup on CSS property regexes; add …
…CSS3 `transform` and `transform-origin` properties; add associated `translation-value`, `translation-function`, and `angle` macros. -dakta
4fa0754
@chromakode

Sweeeeeet. Will review ASAP. Thanks!

@Dakta
Dakta commented Mar 12, 2012

I tested the REGEXes locally, but don't have a Reddit install to make sure the whole thing works. Once I get a working install, I'll see about updating cssutils. While I'm doing that, I'll also refactor the sortof hacky custom macro/properties setup you've got now, as cssutils supports easy addition of custom profiles. The custom macros and properties are even in the right format already, so I just have to call a function and pass the variables. :)

Happy to help.

@chromakode

That would be totally awesome. <3

Let me know our there's anything I can do to help you get rolling!

@chromakode chromakode commented on the diff Mar 13, 2012
r2/r2/lib/cssfilter.py
@@ -74,6 +75,10 @@
'single-text-shadow': r'({color}\s+)?{length}\s+{length}(\s+{length})?|{length}\s+{length}(\s+{length})?(\s+{color})?',
'box-shadow-pos': r'{length}\s+{length}(\s+{length})?(\s+{length})?',
+
+ # http://www.w3.org/TR/css3-2d-transforms/
+ 'translation-value': r'{length}|{percentage}',
+ 'transform-function': r'matrix({num},{w}{num},{w}{num},{w}{num},{w}{num},{w}{num})|translate({translation-value}(,{w}{translation-value})?)|translateX({translation-value})|translateY({translation-value})|scale({num}(,{w}{num})?)|scaleX({num})|scaleY({num})|rotate({angle})|skewX({angle})|skewY({angle})',
@chromakode
chromakode added a note Mar 13, 2012

Nice! Nitpick: should you allow/consume {w} whitespace before the commas as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@chromakode chromakode commented on the diff Mar 13, 2012
r2/r2/lib/cssfilter.py
@@ -88,7 +93,7 @@
'background': r'{bg-gradients}',
'background-image': r'{bg-gradients}',
'background-color': r'{color}',
- 'background-position': r'(({percentage}|{length}){0,3})?\s*(top|center|left)?\s*(left|center|right)?',
+ 'background-position': r'(({percentage}|{length}){0,3})?{w}(top|center|left)?{w}(left|center|right)?',
@chromakode
chromakode added a note Mar 13, 2012

Could you please split cleanup into a separate commit?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@chromakode chromakode commented on the diff Mar 13, 2012
r2/r2/lib/cssfilter.py
@@ -96,6 +101,10 @@
'border-bottom-right-radius': r'{border-radius}',
'border-bottom-left-radius': r'{border-radius}',
'border-top-left-radius': r'{border-radius}',
+
+ # http://www.w3.org/TR/css3-2d-transforms/
+ 'transform': r'none|({transform-function}({w}{transform-function})*)',
@chromakode
chromakode added a note Mar 13, 2012

Are the outer parens (grouping) necessary on the right side of the |?

@Dakta
Dakta added a note Mar 13, 2012

No. Removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@chromakode chromakode commented on the diff Mar 13, 2012
r2/r2/lib/cssfilter.py
@@ -96,6 +101,10 @@
'border-bottom-right-radius': r'{border-radius}',
'border-bottom-left-radius': r'{border-radius}',
'border-top-left-radius': r'{border-radius}',
+
+ # http://www.w3.org/TR/css3-2d-transforms/
+ 'transform': r'none|({transform-function}({w}{transform-function})*)',
+ 'transform-origin': r'(top|bottom)|(({percentage}|{length}|left|center|right)({w}({percentage}|{length}|top|center|bottom))?)|((center|(left|right)({w}({percentage}|{length}))?)&&(center|(top|bottom)({w}({percentage}|{length}))?))',
@chromakode
chromakode added a note Mar 13, 2012

I believe the "&&" in the spec specifies that the two related clauses must be present together; not in the syntax literally. Similar to above, can you remove some of the outer groups?

@Dakta
Dakta added a note Mar 14, 2012

Actually, && is how you specify that two regex groups must both apply for it to pass, which is what the spec implies.

@chromakode
chromakode added a note Mar 14, 2012

Sorry, I'm not sure I follow. I can't find && in the Python regex documentation, and running re.match("(a)&&(b)", "ab") == None. Does it not suffice to include both groups, one after the other? Is the && intended to be in the CSS? Also, it looks like you may need some more {w}s to separate the parameters. For instance, in testing I found that "center&&center" is matched while center && center isn't.

Looking forward to getting this out and seeing what people do with it. :)

@Dakta
Dakta added a note Mar 15, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@spladug
reddit member
spladug commented Jul 15, 2013

I'm going to close this because it has been hanging around so long. Please feel free to open a new one if you have an updated version of the patch. Thanks!

@spladug spladug closed this Jul 15, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.