Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

highlight filter/test broken #78

Closed
hakase opened this Issue · 10 comments

3 participants

@hakase

Basically nothing about the highlight filter seems to work.

Running hamlpy_test.py you get:

Ran 28 tests in 0.004s
OK

When in fact test_pygments_filter should fail but the test doesn't seem to be run due to indentation problems in hamlpy_test.py, there's mixed tabs and spaces. After fixing that I get:

======================================================================
FAIL: test_pygments_filter (__main__.HamlPyTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "hamlpy_test.py", line 213, in test_pygments_filter
    eq_(html, result)
  File "/home/lashni/dev/karasu/lib/python2.7/site-packages/nose/tools.py", line 31, in eq_
    assert a == b, msg or "%r != %r" % (a, b)
AssertionError: '<div class="highlight"><pre><span class="k">print</span> <span class="err">&quot;</span><span class="n">Hello</span><span class="p">,</span> <span class="n">HamlPy</span> <span class="n">World</span><span class="o">!</span></pre></div>' != ':highlight\nprint "Hello, HamlPy World!\n'

----------------------------------------------------------------------
Ran 29 tests in 0.005s
FAILED (failures=1)

Looking at PygmentsFilterNode within nodes.py:

from pygments import highlight
from pygments.formatters import HtmlFormatter
from pygments.lexers import guess_lexer, guess_lexer_for_filename


class PygmentsFilterNode(FilterNode):
    def render(self):
        output = self.spaces
        output += highlighter(self.haml, guess_lexer(self.haml), HtmlFormatter())
        return output

guess_lexer_for_filename is an unused import, output+= highlighter should be highlight.

After making these changes it's still broken, I'm currently working on it. Will update later.

@hakase

in nodes.py create_node function, fixing the indentation for the following gets me a bit further:

    if stripped_line == PYGMENTS_FILTER:
        return PygmentsFilterNode(haml_line)

test code:

from hamlpy import hamlpy


haml = ":highlight\nprint 'hi'"
hamlParser = hamlpy.Compiler()
result = hamlParser.process(haml)

print result
<div class="highlight"><pre><span class="p">:</span><span class="n">highlight</span>
</pre></div>
print 'hi'

At the moment I'm a bit lost trying to grep the rest of the code to figure out why PygmentsFilterNode never has any internal_nodes, it only seems to have access to the highlight tag itself and not the content. I'm sure I'm missing something obvious.

@hakase

Yep, my test code wasn't indented properly.

I think this works now...

from hamlpy import hamlpy


haml = ":highlight\n    print 'hi'"
hamlParser = hamlpy.Compiler()
result = hamlParser.process(haml)

print result
class PygmentsFilterNode(FilterNode):
    def render(self):
        text = "".join((''.join((node.spaces, node.haml,'\n')) for node in self.internal_nodes))
        output = highlight(text, guess_lexer(text), HtmlFormatter())
        return output
<div class="highlight"><pre>    <span class="n">print</span> <span class="s">&#39;hi&#39;</span>
</pre></div>

I'll play with it some more and submit a pull request tomorrow if I haven't overlooked anything obvious.

@hakase

atm I can't figure out how to reconstruct the nodes so that the correct number of blank lines are preserved within the pre block, anyone have any hints?

Alternatively I would have thought there'd be a way to access the raw haml block prior to being stripped of whitespace but I can't find it.

@bcoughlan
Collaborator

Is the problem that it's not leaving line breaks after each line? Do you have any HAML example that demonstrates the problem

@hakase

It's not preserving any blank lines within a pre. From my reading of the haml docs, they'd use :preserve or ~ to keep whitespace and line breaks within a tag but neither seem to be present in hamlpy.

%pre
    from flask import Flask

    app = Flask(__name__)

    @app.route("/")
    def hello():
        return "Hello World!"


    if __name__ == "__main__":
        app.run()

%p Paragraph
<pre>
    from flask import Flask
    app = Flask(__name__)
    @app.route("/")
    def hello():
        return "Hello World!"
    if __name__ == "__main__":
        app.run()
</pre>
<p>hi there</p>
@bcoughlan
Collaborator

Hey, just got a chance to look at this.

Blank lines are removed in nodes.py's create_node() with the lines:

    if not stripped_line:
        return None

Essentially this causes them not to be added to the list of nodes, which is why blank lines are removed from output.

You were saying that haml has :preserve and ~. Can you think of any reason why hamlpy should not preserve by default (I can't)? Preserving by default would be as simple as changing the above to

    if not stripped_line:
        return HamlNode(haml_line)

and fixing the four tests that fail due to the empty lines.

@hakase

Preserve by default seems logical to me, can't think of a case where you'd want the opposite.

@bcoughlan
Collaborator

Had a proper look at this. This is trickier than it looks, if you have

%pre
  Foo
(Blank line, no spaces)
  Bar

If the blank line has no indentation HamlPy thinks it's the end of the <pre> tag.

There are a few ways to fix this, but I'll have to do some more thinking to decide on the best way.

@czartur

I've created a simple patch, that adds a pipe support (much like shpaml)

here's the commit:

czartur/HamlPy@707d3c3

and it works exactly like described here:
https://github.com/czartur/HamlPy/blob/master/reference.md#dropping-leading-whitespace

@bcoughlan
Collaborator

I've been too busy lately to have a proper look at this issue.

@czartur It seems a bit cumbersome to do it this way. Since the purpose of haml is to be easier than html, I wouldn't like to see something added that is more difficult to do than HTML.

There may be a simpler hack in hamlpy.py (where multi-line text is merged into one line) where we could append \n's to a HamlNode.

Another possibility would be to get the indentation of a blank line to match the indentation of the previous line, which would solve the problem I mentioned previously. In practice this would require some refactoring in create_node so that the previous node is passed in.

@bcoughlan bcoughlan referenced this issue from a commit in bcoughlan/HamlPy
@barrycoughlan barrycoughlan Preserve whitespace by default (#78) (also fixes #81). Experimental f…
…ix, need to refactor before making pull request
aa7386b
@bcoughlan bcoughlan closed this
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.