Proper element and filter parsing #40

rowanseymour · 2016-12-07T10:51:48Z

Currently the node parser works on a line by line basis which leads to following issues:

To determine if an element node spans multiple lines, it counts the number of { and } characters. You can break things by putting a brace into an attribute value. People don't often use { characters not in pairs, but now that we also support (...) style attribute dictionaries, chances of things breaking are a bit higher.
Filters are parsed as a single line node, with each line of nested content being parsed like any other node. This leads to inefficiency as something like :plain\n %abc, causes the %abc line to be parsed as an element even tho it's just a line of plain text

This PR reworks the parsing of nodes so that:

Multiline elements are parsed by a proper element parser which reads lines until it has a complete element
Filter nodes are parsed with all nested lines as their text content, rather than as child nodes.
Filters become simple functions in their own module, and leverage textwrap.dedent for removing indentation.

Performance-wise new code seems to be about ~40% faster.

Fixes #38, #39 and #41

…unctions

…g on how lines are split

…racter } or )

rowanseymour · 2016-12-07T10:52:45Z

hamlpy/hamlpy.py

-
-            if not root.parent_of(HamlNode(line, self)).inside_filter_node():
-                if line.count('{') - line.count('}') == 1:
-                    start_multiline = line_number  # for exception handling


Tracking line numbers would be useful but this line doesn't actually do anything

rowanseymour · 2016-12-07T10:53:12Z

hamlpy/hamlpy.py

-            node_lines = line
-
-            if not root.parent_of(HamlNode(line, self)).inside_filter_node():
-                if line.count('{') - line.count('}') == 1:


This where things break if you have {} characters in attribute values

rowanseymour · 2016-12-07T10:54:59Z

hamlpy/parser/attributes.py

@@ -144,11 +130,17 @@ def read_attribute_dict(stream):
    """
    data = OrderedDict()

-    start, terminator = stream.text[0], stream.text[-1]


Previously this code assumed the stream only contained an attribute dictionary - now it contains the entire haml document

coveralls · 2016-12-07T10:55:27Z

Coverage decreased (-0.1%) to 99.885% when pulling aabb4b0 on multiline_elements into 39c38ac on master.

rowanseymour · 2016-12-07T10:56:34Z

hamlpy/parser/filters.py

+
+
+def stylus(content, indent, options):
+    return indent + '<style type=%(attr_wrapper)stext/stylus%(attr_wrapper)s>\n' \


There are some inconsistencies with now indentation is handled by different filters. I've tried to preserve some of those inconsistencies for now so that the template tests work as is

rowanseymour · 2016-12-07T10:58:34Z

hamlpy/test/test_nodes.py


        start.add_node(one)
        start.add_node(two)
        start.add_node(three)

        self.assertEqual(3, len(start.children))

-    def test_node_parent_function(self):


parent_of was being used to figure out of a node belonged to filter node that no longer happens and this is no longer needed

coveralls · 2016-12-07T12:33:56Z

Coverage remained the same at 100.0% when pulling 928dc83 on multiline_elements into 39c38ac on master.

coveralls · 2016-12-07T12:58:10Z

Coverage remained the same at 100.0% when pulling 52a35fd on multiline_elements into 39c38ac on master.

rowanseymour · 2016-12-07T13:09:41Z

hamlpy/test/templates/filters.html

@@ -24,3 +24,4 @@
 3
 4
 5
+


This is one of a few cases where we don't match the original output exactly because of an additional new line. IMO this new line is correct (it's coming from the python print) and shouldn't break anyone's stuff.

…or empty lines in attribute alues which are Haml

rowanseymour · 2016-12-07T14:34:51Z

hamlpy/test/test_attributes.py

@@ -97,6 +93,7 @@ def test_parse(self):
                'class':
                    - if forloop.first
                        link-first
+


This blank line tests #41

coveralls · 2016-12-07T14:50:10Z

Coverage decreased (-0.1%) to 99.885% when pulling bc7ced0 on multiline_elements into 39c38ac on master.

…t coverage back to 100%

coveralls · 2016-12-07T15:51:27Z

Coverage remained the same at 100.0% when pulling 5e00da1 on multiline_elements into 39c38ac on master.

coveralls · 2016-12-07T15:51:27Z

Coverage remained the same at 100.0% when pulling 5e00da1 on multiline_elements into 39c38ac on master.

coveralls · 2016-12-07T15:51:27Z

Coverage remained the same at 100.0% when pulling 5e00da1 on multiline_elements into 39c38ac on master.

rowanseymour added 10 commits December 6, 2016 09:04

Add read_line parser function and unit tests for all generic parser f…

d9c0d59

…unctions

Get rid of process_lines which isn't consistent with process dependin…

8568559

…g on how lines are split

Use read_line to loop over each line in Haml passed to compiler

208ed81

Rework read_attribute_dict to not assume stream ends with closing cha…

9965d81

…racter } or )

Proper parsing of element nodes

500fc41

Rework node parsing (WIP)

0f8df3a

Reworked filter nodes so thet don't have children

47004f3

Simplify node classes removing no longer needed functionality

429a493

Rename consume_whitespace to read_whitespace as that is what it does now

25e2131

Change some template tests to use HTML style attributes

aabb4b0

rowanseymour added bug enhancement labels Dec 7, 2016

rowanseymour self-assigned this Dec 7, 2016

rowanseymour commented Dec 7, 2016

View reviewed changes

More simplication of node classes and fix missing test coverage

928dc83

rowanseymour added 2 commits December 7, 2016 14:36

Use py.test asserts and remove old TODO comment

dfda1ef

Removed cruft

52a35fd

rowanseymour commented Dec 7, 2016

View reviewed changes

Rework read_attribute_value_haml to not use regexes and add support f…

bc7ced0

…or empty lines in attribute alues which are Haml

rowanseymour commented Dec 7, 2016

View reviewed changes

Test parsing attribute value with multiline with a blank line.. to ge…

5e00da1

…t coverage back to 100%

rowanseymour merged commit 63b7b0a into master Dec 8, 2016

rowanseymour deleted the multiline_elements branch December 8, 2016 06:49

This was referenced Dec 8, 2016

Attribute values which are multiline Haml can't have blank lines #41

Closed

Attribute values can't contain braces #39

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Proper element and filter parsing #40

Proper element and filter parsing #40

rowanseymour commented Dec 7, 2016 •

edited

Loading

rowanseymour Dec 7, 2016

rowanseymour Dec 7, 2016

rowanseymour Dec 7, 2016

coveralls commented Dec 7, 2016 •

edited

Loading

rowanseymour Dec 7, 2016

rowanseymour Dec 7, 2016

coveralls commented Dec 7, 2016 •

edited

Loading

coveralls commented Dec 7, 2016 •

edited

Loading

rowanseymour Dec 7, 2016

rowanseymour Dec 7, 2016

coveralls commented Dec 7, 2016 •

edited

Loading

coveralls commented Dec 7, 2016 •

edited

Loading

coveralls commented Dec 7, 2016

coveralls commented Dec 7, 2016



		def stylus(content, indent, options):
		return indent + '<style type=%(attr_wrapper)stext/stylus%(attr_wrapper)s>\n' \

@@ @@ -24,3 +24,4 @@ @@

Proper element and filter parsing #40

Proper element and filter parsing #40

Conversation

rowanseymour commented Dec 7, 2016 • edited Loading

rowanseymour Dec 7, 2016

Choose a reason for hiding this comment

rowanseymour Dec 7, 2016

Choose a reason for hiding this comment

rowanseymour Dec 7, 2016

Choose a reason for hiding this comment

coveralls commented Dec 7, 2016 • edited Loading

rowanseymour Dec 7, 2016

Choose a reason for hiding this comment

rowanseymour Dec 7, 2016

Choose a reason for hiding this comment

coveralls commented Dec 7, 2016 • edited Loading

coveralls commented Dec 7, 2016 • edited Loading

rowanseymour Dec 7, 2016

Choose a reason for hiding this comment

rowanseymour Dec 7, 2016

Choose a reason for hiding this comment

coveralls commented Dec 7, 2016 • edited Loading

coveralls commented Dec 7, 2016 • edited Loading

coveralls commented Dec 7, 2016

coveralls commented Dec 7, 2016

rowanseymour commented Dec 7, 2016 •

edited

Loading

coveralls commented Dec 7, 2016 •

edited

Loading

coveralls commented Dec 7, 2016 •

edited

Loading

coveralls commented Dec 7, 2016 •

edited

Loading

coveralls commented Dec 7, 2016 •

edited

Loading

coveralls commented Dec 7, 2016 •

edited

Loading