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

Fix Markdown br bug #138

Merged
merged 3 commits into from
Jul 29, 2013
Merged

Fix Markdown br bug #138

merged 3 commits into from
Jul 29, 2013

Conversation

boscoh
Copy link
Contributor

@boscoh boscoh commented Jun 14, 2013

In the :markdown filter, the markdown for implicit <br> tag is broken.

The reason is that markdown recognises a line with <br> by two trailing spaces.

However, trailing spaces are stripped when PlaintextNode.haml is read.

To fix: when the text is reconstructed to send to the Markdown filter, instead of using the PlaintextNode.haml field, PlaintextNode.raw_haml.lstrip() is used, which keeps the trailing spaces.

@noelbush-xx
Copy link
Collaborator

Thanks very much! Any chance of writing a quick unit test for this? Would be much appreciated.

@boscoh
Copy link
Contributor Author

boscoh commented Jun 17, 2013

I rewrote the markdown test in the markdown_br_fix branch to add the
check.

@noelbush-xx
Copy link
Collaborator

Thanks for updating the test. It doesn't seem to be passing for me, though:

FAIL: test_filters_markdown (hamlpy.test.template_compare_test.TestTemplateCompare)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/nsh/projects/HamlPy/hamlpy/test/template_compare_test.py", line 47, in test_filters_markdown
self._compare_test_files('filtersMarkdown')
  File "/home/nsh/projects/HamlPy/hamlpy/test/template_compare_test.py", line 124, in _compare_test_files
eq_(parsed, html)
AssertionError: u'<p>hello</p>\n<p>no paragraph</p>\n<p>line break  </p>\n<p>follow</p>\n<p>New paragraph</p>\n<pre><code>code block\n</code></pre>' != '<p>hello\nno paragraph</p>\n<p>line break<br />\nfollow</p>\n<p>New paragraph</p>\n<pre><code>code block\n</code></pre>'
-------------------- >> begin captured stdout << ---------------------

HTML (actual): 
1. <p>hello</p>
2. <p>no paragraph</p>
3. <p>line break  </p>
4. <p>follow</p>
5. <p>New paragraph</p>
6. <pre><code>code block
7. </code></pre>
Difference begins at line 1 column 9
HTML (actual, len=12)   : <p>hello</p>
HTML (expected, len= 8) : <p>hello
Character code (actual)  : 60 (<)
Character code (expected): 10 (
)

--------------------- >> end captured stdout << ----------------------

@boscoh
Copy link
Contributor Author

boscoh commented Jun 21, 2013

The origin of that was a bit subtle. Turns out hamlpy.py reads the input file using read().splitlines(), whereas the template tests uses readlines(). This gives different line endings w.r.t. to '\n'. Now the markdown filter must explicitly check for the line endings, to remove it.

noelbush-xx added a commit that referenced this pull request Jul 29, 2013
Fix Markdown br bug
Thanks to @boscoh for the following:
In the `:markdown` filter, the markdown for implicit `<br>` tag is broken.

The reason is that markdown recognises a line with  `<br>` by two trailing spaces.

However, trailing spaces are stripped when `PlaintextNode.haml` is read.

To fix: when the text is reconstructed to send to the Markdown filter, instead of using the `PlaintextNode.haml` field, `PlaintextNode.raw_haml.lstrip()` is used, which keeps the trailing spaces.
@noelbush-xx noelbush-xx merged commit fe3ba8b into jessemiller:master Jul 29, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants