Skip to content
This repository has been archived by the owner on Aug 17, 2023. It is now read-only.

Fixed problem parsing boolean attributes in html style attribute dicts #42

Merged
merged 1 commit into from
Dec 12, 2016
Merged

Fixed problem parsing boolean attributes in html style attribute dicts #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ This is the first major release and there are some potentially breaking changes

* Refactor of parsing code giving ~40% performance improvement
* Added support for HTML style attribute dictionaries, e.g. `%span(foo="bar")`
* Improved error reporting from parser to help find problems in your templates
* Fixed attribute values not being able to include braces (https://github.com/nyaruka/django-hamlpy/issues/39)
* Fixed attribute values which are Haml not being able to have blank lines (https://github.com/nyaruka/django-hamlpy/issues/41)
* Fixed sequential `with` tags ended up nested (https://github.com/nyaruka/django-hamlpy/issues/23)
Expand Down
14 changes: 10 additions & 4 deletions hamlpy/parser/attributes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from collections import OrderedDict

from .generic import ParseException, read_whitespace, read_symbol, read_number, read_quoted_string, read_word
from .generic import peek_indentation, read_line, STRING_LITERALS
from .generic import peek_indentation, read_line, STRING_LITERALS, WHITESPACE_CHARS

LEADING_SPACES_REGEX = regex.compile(r'^\s+', regex.V1 | regex.MULTILINE)

Expand Down Expand Up @@ -108,11 +108,15 @@ def read_attribute(stream, assignment_symbols, entry_separator, terminator):
if not key:
raise ParseException("Empty attribute key.", stream)

assignment_prefixes = [s[0] for s in assignment_symbols]

ch = stream.text[stream.ptr]
if ch not in WHITESPACE_CHARS and ch not in (entry_separator, terminator) and ch not in assignment_prefixes:
raise ParseException("Unexpected \"%s\"." % ch, stream)

read_whitespace(stream)

if stream.text[stream.ptr] in (entry_separator, terminator):
value = None
else:
if stream.text[stream.ptr] in assignment_prefixes:
read_symbol(stream, assignment_symbols)

read_whitespace(stream)
Expand All @@ -125,6 +129,8 @@ def read_attribute(stream, assignment_symbols, entry_separator, terminator):
value = read_attribute_value_list(stream)
else:
value = read_attribute_value(stream)
else:
value = None

return key, value

Expand Down
13 changes: 10 additions & 3 deletions hamlpy/test/test_attributes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,16 @@ def test_read_attribute_dict(self):
assert dict(self._parse("{class: 'test\u1234'}")) == {'class': 'test\u1234'}

# html style dicts
assert dict(self._parse("()")) == {}
assert dict(self._parse("( )")) == {}

assert dict(self._parse(
"(disabled :class='test' data-number = 123\n 'foo'=\"bar\")"
)) == {'disabled': None, 'class': 'test', 'data-number': '123', 'foo': 'bar'}

assert dict(self._parse(
"(:class='test' :data-number = 123\n foo=\"bar\")"
)) == {'class': 'test', 'data-number': '123', 'foo': 'bar'}
"(:class='test' data-number = 123\n 'foo'=\"bar\" \t disabled)"
Copy link

Choose a reason for hiding this comment

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

are colons legal syntax? I would expect this to blow up..

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like for {:class} dicts it's the actual Haml syntax, but they're not used for HTML-style dicts http://haml.info/docs/yardoc/file.REFERENCE.html#attributes_

Currently we just ignore them, but seems it should be an error to use one in an HTML-style dict

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually looks like they're only used with the => syntax (which we do support), so it should be an error to do...

%span{:class:"test"}
%span(:class="test")

but not..

%span{:class=>"test"}

Copy link

Choose a reason for hiding this comment

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

WTF that is the ugliest syntax I've ever seen, what an abomination! Wonder what the reasoning around that is.

My bias for anything like this is to be strict instead of ignoring things that look goofy.

Copy link
Member Author

Choose a reason for hiding this comment

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

That was how you made a hash (dict) in Ruby before 1.9 apparently http://effectif.com/ruby/update-your-project-for-ruby-19-hash-syntax

Agreed on disallowing this - have recorded that at #43

Copy link

Choose a reason for hiding this comment

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

HAML is so weird.

)) == {'disabled': None, 'class': 'test', 'data-number': '123', 'foo': 'bar'}

def test_empty_attribute_raises_error(self):
# empty quoted string
Expand Down Expand Up @@ -149,7 +156,7 @@ def test_mixing_ruby_and_html_syntax_raises_errors(self):
self._parse("(class='test', foo = 'bar')")

# use : in HTML style dict
with self.assertRaisesRegexp(ParseException, "Expected \"=\". @ \"\(class:\" <-"):
with self.assertRaisesRegexp(ParseException, "Unexpected \":\". @ \"\(class:\" <-"):
self._parse("(class:'test')")

# use => in HTML style dict
Expand Down