Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix bug with gathering list item lines #229

Merged
merged 1 commit into from
Dec 30, 2015
Merged

Fix bug with gathering list item lines #229

merged 1 commit into from
Dec 30, 2015

Conversation

rtfb
Copy link
Collaborator

@rtfb rtfb commented Dec 26, 2015

This problem has been lurking since the very early days, apparently introduced with commit 583b3c5. Strange that nobody stumbled upon it for so long.

Instead of swallowing an empty line and then reintroducing it back again in certain cases, collect the list item body in an unaltered form and let the recursive parsing call sort things out.

Fixes issue #228.

Instead of swallowing an empty line and then reintroducing it back again
in certain cases, collect the list item body in an unaltered form and
let the recursive parsing call sort things out.

Fixes issue #228.
@dmitshur
Copy link
Collaborator

This is an awesome fix for blackfriday! 👏

I am surprised too. I suppose having more than 1 blank line in real world documents is pretty rare, but it wasn't correct even in that case (there was an extra blank line rendered). It's probably because most other Markdown renderers seem to have this bug too, so that might've been a deterrent for reporting this.

I'm not closely familiar with implementation details of (*parser).listItem, so all I can go on are that previous and new tests pass, and the code change looks reasonable with nothing that stands out to me.

LGTM based on that.

miekg added a commit to miekg/mmark that referenced this pull request Dec 30, 2015
rtfb added a commit that referenced this pull request Dec 30, 2015
Fix bug with gathering list item lines
@rtfb rtfb merged commit c8875c0 into master Dec 30, 2015
@dmitshur dmitshur deleted the issue-228 branch December 30, 2015 20:40
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants