Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support shorthand property compression. #24

Merged
merged 1 commit into from
Aug 4, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 73 additions & 0 deletions tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from toronado import (
Properties,
Rule,
compress_box_property,
expand_shorthand_box_property,
from_string,
inline,
Expand All @@ -24,6 +25,68 @@ class TestCase(Exam, unittest.TestCase):
pass


def test_compress_box_property():
compress = compress_box_property('margin', 'margin-{}')

assert compress({
'margin-top': '1px',
'margin-right': '1px',
'margin-bottom': '1px',
'margin-left': '1px',
}) == {
'margin': '1px',
}

assert compress({
'margin-top': '1px',
'margin-right': '2px',
'margin-bottom': '1px',
'margin-left': '2px',
}) == {
'margin': '1px 2px',
}

assert compress({
'margin-top': '1px',
'margin-right': '2px',
'margin-bottom': '3px',
'margin-left': '2px',
}) == {
'margin': '1px 2px 3px',
}

assert compress({
'margin-top': '1px',
'margin-right': '2px',
'margin-bottom': '3px',
'margin-left': '4px',
}) == {
'margin': '1px 2px 3px 4px',
}

assert compress({
'margin-top': '1px',
'margin-bottom': '1px',
}) == {
'margin-top': '1px',
'margin-bottom': '1px',
}

assert compress({
'margin-top': '1px',
'margin-right': '1px',
'margin-bottom': '1px',
'margin-left': '1px',
'other-property': 'foo',
}) == {
'margin': '1px',
'other-property': 'foo',
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may wanna have another test case to cove

{
    'margin-top': '0px',
    'margin-right': '2px',
    'margin-bottom': '0',
    'margin-left': '2px',
}

Basically when the value is 0, you should still collapse that regardless of the unit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, will add this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a little tricky, because to be reliably implemented it'd need to support:

I opened another ticket to take care of this later: GH-25

properties = {}
assert compress(properties) is properties


def test_expand_shorthand_box_property():
expand = expand_shorthand_box_property('margin-{}')

Expand Down Expand Up @@ -113,6 +176,16 @@ def test_serializes_to_attribute_string(self):

self.assertIn('%s' % (properties,), expected)

def test_compresses_shorthand_properties(self):
properties = Properties({
'margin-top': '10px',
'margin-right': '10px',
'margin-bottom': '10px',
'margin-left': '10px',
})

assert '%s' % (properties,) == 'margin: 10px'

def test_from_string(self):
properties = Properties.from_string('color: red; font-weight: bold')
self.assertEqual(properties, {
Expand Down
67 changes: 52 additions & 15 deletions toronado/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,12 @@
logger = logging.getLogger(__name__)


def expand_box_property_names(template):
return list(map(template.format, ('top', 'right', 'bottom', 'left')))


def expand_shorthand_box_property(template):
sides = ('top', 'right', 'bottom', 'left')
names = expand_box_property_names(template)

def expand_property(value):
bits = value.split()
Expand All @@ -44,18 +48,11 @@ def expand_property(value):
else:
raise ValueError('incorrect number of values for box rule: %s' % size)

return {template.format(side): value for side, value in zip(sides, result)}
return {name: value for name, value in zip(names, result)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this basically dict(zip(names, result))?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, oops.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed here: 8d6bc29


return expand_property


rewrite_map = {
'margin': expand_shorthand_box_property('margin-{}'),
'padding': expand_shorthand_box_property('padding-{}'),
'border-width': expand_shorthand_box_property('border-{}-width'),
}


def warn_unsupported_shorthand_property(property):
def expand_property(value):
logger.warning(
Expand All @@ -69,6 +66,35 @@ def expand_property(value):
return expand_property


def compress_box_property(shorthand, template):
names = expand_box_property_names(template)

def compress_property(value):
if not set(value).issuperset(set(names)):
return value

top, right, bottom, left = map(value.pop, names)

if top == right == bottom == left:
value[shorthand] = top
elif top == bottom and right == left:
value[shorthand] = '{} {}'.format(top, right)
elif right == left:
value[shorthand] = '{} {} {}'.format(top, right, bottom)
else:
value[shorthand] = '{} {} {} {}'.format(top, right, bottom, left)

return value

return compress_property


shorthand_box_properties = {
'margin': 'margin-{}',
'padding': 'padding-{}',
'border-width': 'border-{}-width',
}

unsupported_shorthand_properties = (
'animation',
'background',
Expand All @@ -87,12 +113,19 @@ def expand_property(value):
)


expansion_rewrite_map = {}
property_processors = []

for property, template in shorthand_box_properties.items():
expansion_rewrite_map[property] = expand_shorthand_box_property(template)
property_processors.append(compress_box_property(property, template))

for property in unsupported_shorthand_properties:
rewrite_map[property] = warn_unsupported_shorthand_property(property)
expansion_rewrite_map[property] = warn_unsupported_shorthand_property(property)


def rewrite_property(property):
result = rewrite_map.get(
def expand_property(property):
result = expansion_rewrite_map.get(
property.name,
lambda value: {
property.name: value,
Expand Down Expand Up @@ -122,13 +155,17 @@ def __unicode__(self):
Renders the properties as a string suitable for inclusion as a HTML tag
attribute.
"""
return '; '.join(map(': '.join, self.items()))
value = self.copy()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably unnecessary, but having a __unicode__ method mutate self seemed weird.

for processor in property_processors:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a cool way to do this: http://stackoverflow.com/a/11736942/90297 (don't think it is better than yours, was just curious and wanted to share what I found)

value = processor(value)

return '; '.join(map(': '.join, value.items()))

@classmethod
def from_string(cls, value):
values = {}
for property in CSSStyleDeclaration(value).getProperties():
values.update(rewrite_property(property))
values.update(expand_property(property))
return cls(values)


Expand Down Expand Up @@ -218,7 +255,7 @@ def inline(tree):
for rule in ifilter(is_style_rule, stylesheet_parser.parseString(stylesheet.text)):
properties = {}
for property in rule.style:
properties.update(rewrite_property(property))
properties.update(expand_property(property))

# XXX: This doesn't handle selectors with odd multiple whitespace.
for selector in map(text_type.strip, rule.selectorText.split(',')):
Expand Down