Expand shorthand box properties (margin and padding.)

* Use `CSSStyleDeclaration` for `Properties.from_string` (and remove
  unnecessary test.)
* Remove poorly implemented failing test.
* Clean up imports for `cssutils`.

References GH-19.

tests.py

@@ -6,7 +6,13 @@
 from lxml import etree, html
 from lxml.cssselect import CSSSelector
 
-from toronado import Rule, Properties, inline, from_string
+from toronado import (
+    Properties,
+    Rule,
+    expand_shorthand_box_property,
+    from_string,
+    inline,
+)
 
 try:
     from lxml.html import soupparser
@@ -18,27 +24,42 @@ class TestCase(Exam, unittest.TestCase):
     pass
 
 
+def test_expand_shorthand_box_property():
+    assert expand_shorthand_box_property('margin', '1px') == {
+        'margin-top': '1px',
+        'margin-right': '1px',
+        'margin-bottom': '1px',
+        'margin-left': '1px',
+    }
+
+    assert expand_shorthand_box_property('margin', '1px 2px') == {
+        'margin-top': '1px',
+        'margin-right': '2px',
+        'margin-bottom': '1px',
+        'margin-left': '2px',
+    }
+
+    assert expand_shorthand_box_property('margin', '1px 2px 3px') == {
+        'margin-top': '1px',
+        'margin-right': '2px',
+        'margin-bottom': '3px',
+        'margin-left': '2px',
+    }
+
+    assert expand_shorthand_box_property('margin', '1px 2px 3px 4px') == {
+        'margin-top': '1px',
+        'margin-right': '2px',
+        'margin-bottom': '3px',
+        'margin-left': '4px',
+    }
+
+
 class RuleTestCase(TestCase):
     def test_compares_by_specificity(self):
         self.assertGreater(Rule('#main'), Rule('div'))
         self.assertEqual(Rule('div'), Rule('p'))
         self.assertLess(Rule('div'), Rule('div.container'))
 
-    def test_combine_applies_shorthand(self):
-        properties = Rule.combine((
-            Rule('h1', {
-                'padding': '0 10px',
-            }),
-            Rule('h1#primary', {
-                'padding-bottom': '20px',
-            }),
-        ))
-
-        self.assertIsInstance(properties, Properties)
-        self.assertEqual(properties, {
-            'padding': '0 10px 0 20px',
-        })
-
     def test_combine_respects_specificity_rules(self):
         properties = Rule.combine((
             Rule('h1', {
@@ -79,11 +100,12 @@ def test_from_string(self):
             'font-weight': 'bold',
         })
 
-    def test_from_string_cleans_whitespace(self):
-        properties = Properties.from_string('color : red;\nfont-weight: bold ;')
+        properties = Properties.from_string('padding: 0 10px')
         self.assertEqual(properties, {
-            'color': 'red',
-            'font-weight': 'bold',
+            'padding-top': '0',
+            'padding-right': '10px',
+            'padding-bottom': '0',
+            'padding-left': '10px',
         })
 
 

toronado/__init__.py

@@ -1,13 +1,18 @@
 from __future__ import absolute_import, unicode_literals, print_function
 
-import cssutils
 import itertools
 import logging
 import sys
 
 from collections import defaultdict
 from lxml import html
 from lxml.cssselect import CSSSelector
+from cssutils import CSSParser
+from cssutils.css import (
+    CSSRule,
+    CSSStyleDeclaration,
+    Selector,
+)
 
 PY3 = sys.version_info[0] == 3
 
@@ -19,6 +24,89 @@
     ifilter = __import__('itertools').ifilter
 
 
+logger = logging.getLogger(__name__)
+
+
+def expand_shorthand_box_property(name, value):
+    bits = value.split()
+    size = len(bits)
+    if size == 1:
+        result = (bits[0],) * 4
+    elif size == 2:
+        result = (bits[0], bits[1],) * 2
+    elif size == 3:
+        result = (bits[0], bits[1], bits[2], bits[1])
+    elif size == 4:
+        result = tuple(bits)
+    else:
+        raise ValueError('incorrect number of values for box rule: %s' % size)
+
+    sides = ('top', 'right', 'bottom', 'left')
+    return {'%s-%s' % (name, side): value for side, value in zip(sides, result)}
+
+
+def rewrite_margin_property_value(value):
+    return expand_box_rule('margin', value)
+
+
+def rewrite_padding_property_value(value):
+    return expand_box_rule('padding', value)
+
+
+rewrite_map = {
+    'margin': expand_shorthand_box_property,
+    'padding': expand_shorthand_box_property,
+}
+
+
+def warn_unsupported_shorthand_property(name, value):
+    logger.warning(
+        "CSS shorthand syntax expansion is not supported for %r. Mixing "
+        "shorthand and specific property values (e.g. `font` and `font-size`) "
+        "may lead to unexpected results.",
+        name,
+    )
+    return {name: value}
+
+
+unsupported_shorthand_properties = (
+    'animation',
+    'background',
+    'border',
+    'border-bottom',
+    'border-color',
+    'border-left',
+    'border-radius',
+    'border-right',
+    'border-style',
+    'border-top',
+    'border-width',
+    'font',
+    'list-style',
+    'transform',
+    'transition',
+)
+
+
+for property in unsupported_shorthand_properties:
+    rewrite_map[property] = warn_unsupported_shorthand_property
+
+
+def rewrite_property(property):
+    result = rewrite_map.get(
+        property.name,
+        lambda name, value: {
+            name: value,
+        }
+    )(property.name, property.value)
+
+    if property.priority:
+        for key, value in result.items():
+            result[key] = "%s ! %s" % (value, property.priority)
+
+    return result
+
+
 class Properties(dict):
     """
     A container for CSS properties.
@@ -39,11 +127,10 @@ def __unicode__(self):
 
     @classmethod
     def from_string(cls, value):
-        rules = [
-            map(text_type.strip, property.split(':'))
-            for property in value.split(';') if property
-        ]
-        return Properties(rules)
+        values = {}
+        for property in CSSStyleDeclaration(value).getProperties():
+            values.update(rewrite_property(property))
+        return cls(values)
 
 
 class Rule(object):
@@ -59,7 +146,7 @@ def __init__(self, selector, properties=None):
             self.properties.update(properties)
 
         # NOTE: This should be available by `CSSSelector`?
-        self.specificity = cssutils.css.Selector(selector).specificity
+        self.specificity = Selector(selector).specificity
 
     def __repr__(self):
         return '<Rule: %s>' % self.selector.css
@@ -96,7 +183,7 @@ def is_style_rule(rule):
     """
     Returns if a :class:`cssutils.css.CSSRule` is a style rule (not a comment.)
     """
-    return rule.type == cssutils.css.CSSRule.STYLE_RULE
+    return rule.type == CSSRule.STYLE_RULE
 
 
 def inline(tree):
@@ -113,18 +200,9 @@ def inline(tree):
         </style>
 
     """
-
-    def _prio_value(p):
-        """
-        Format value and priority of a :class:`cssutils.css.Property`.
-        """
-        if p.priority:
-            return "%s ! %s" % (p.value, p.priority)
-        return p.value
-
     rules = {}
 
-    stylesheet_parser = cssutils.CSSParser(log=logging.getLogger('%s.cssutils' % __name__))
+    stylesheet_parser = CSSParser(log=logging.getLogger('%s.cssutils' % __name__))
 
     # Get all stylesheets from the document.
     stylesheets = CSSSelector('style')(tree)
@@ -137,7 +215,10 @@ def _prio_value(p):
             continue
 
         for rule in ifilter(is_style_rule, stylesheet_parser.parseString(stylesheet.text)):
-            properties = dict([(property.name, _prio_value(property)) for property in rule.style])
+            properties = {}
+            for property in rule.style:
+                properties.update(rewrite_property(property))
+
             # XXX: This doesn't handle selectors with odd multiple whitespace.
             for selector in map(text_type.strip, rule.selectorText.split(',')):
                 rule = rules.get(selector, None)