Fix a blank line with partial indent evicting a list item early - #629
Open
afonsojanu wants to merge 1 commit into
Open
Fix a blank line with partial indent evicting a list item early#629afonsojanu wants to merge 1 commit into
afonsojanu wants to merge 1 commit into
Conversation
Reported in commonmark#618: a blank line inside a list item, made up only of spaces that fall short of the item's own indent width, would end the item and push whatever came after out as a sibling paragraph, even though a fully empty blank line or one padded to the full indent width kept the item open just fine. The three cases only diverge when the item's opening line consisted solely of a link reference definition. Once the reference resolves, finalize() frees that now-empty paragraph, and parse_node_item_prefix was using container->first_child == NULL as a proxy for "the opening line was blank", since that's normally the only way an item ends up childless. A reference-only opening line hits the same childless state for an unrelated reason, so a later short blank line got read as "this item never had anything in it" and the item closed prematurely. A blank line with zero or full-width indent happened to avoid the branch in parse_node_item_prefix that checks first_child at all, which is why only the partial-indent case showed the bug. Added a CMARK_NODE__ITEM_HAD_CONTENT flag, set on the item when a reference-only paragraph belonging to it is freed, and checked alongside first_child in parse_node_item_prefix. Added a regression test to test/regression.txt using the exact shape from the issue. Ran the full local suite (api tests, CommonMark spec tests, smart punctuation tests, and the regression suite) before and after: all green after, and the new case fails without the fix, confirmed by temporarily reverting just the blocks.c change and rebuilding.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #618.
A blank line inside a list item made up only of spaces short of the item's own indent width would end the item early and push whatever came after out as a sibling paragraph, even though a fully empty blank line or one padded out to the full indent width kept the item open.
The three shapes from the issue only diverge when the item's opening line is a link reference definition on its own. Once the reference resolves,
finalize()frees that now-empty paragraph, andparse_node_item_prefixwas usingcontainer->first_child == NULLas a stand-in for "the opening line was blank", since normally that's the only way an item ends up childless at this point. A reference-only opening line hits the same childless state for a different reason entirely, so a later short blank line got misread as "this item never had anything in it." A blank line with zero or full-width indent happens to avoid the branch inparse_node_item_prefixthat even looks atfirst_child, which is why only the partial-indent case showed the bug.Added a
CMARK_NODE__ITEM_HAD_CONTENTflag, set on the item when a reference-only paragraph belonging to it gets freed, and checked alongsidefirst_child. Added a regression test using the exact input from the issue.Ran the full local suite (api tests, CommonMark spec tests, smart punctuation tests, regression tests) before and after. Confirmed the new case fails without the
blocks.cchange (reverted it locally, rebuilt, saw the failure) and passes with it restored.