Skip to content

Commit

Permalink
Make list tightness match the reference implementation closer
Browse files Browse the repository at this point in the history
This solves the problem where blank lines in the middle of a list
are attributed to the list itself instead of the item, making
its parent list become spurriously loose.

Given this reduced example, which helps understand the problem
but not reproduce it:

```markdown
 * -

   -

test
```

Consider this subset of the test case:

 1 | * -
   |     ^ first blank line of ListItem
 2 |
   |     ^ second blank line of ListItem
 3 |
   |     ^ the is what the commit changes:
   |       it used to be considered a blank in the List,
   |       but is now also considered part of the ListItem
 4 |   -
   |     ^ second ListItem takes its first blank, just like before
 5 |
   |  ^ as part of finalization, this second blank goes three two ownership changes:
   |    first, the `-` List takes it from its ListItem, because it's the last ListItem,
   |    and, second, the `*` List takes it from its child, because it's at the end
   |
 6 | test
   | ^ lists end here

The change in this commit changes the List to take ownership
of a blank line only if its a trailing blank line at the end
of a list item (like line 5) and never from the middle
(like lines 3 and 2).
  • Loading branch information
notriddle authored and jgm committed Mar 8, 2024
1 parent 72b59ad commit 1ec2293
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 3 deletions.
6 changes: 3 additions & 3 deletions commonmark/src/Commonmark/Blocks.hs
Original file line number Diff line number Diff line change
Expand Up @@ -856,10 +856,10 @@ listItemSpec parseListMarker = BlockSpec
let lidata = fromDyn (blockData ndata)
(ListItemData (BulletList '*') 0 False False)
-- a marker followed by two blanks is just an empty item:
guard $ null (blockBlanks ndata) ||
not (null children)
pos <- getPosition
gobbleSpaces (listItemIndent lidata) <|> 0 <$ lookAhead blankLine
case blockBlanks ndata of
_:_ | null children -> lookAhead blankLine
_ -> () <$ gobbleSpaces (listItemIndent lidata) <|> lookAhead blankLine
return (pos, node)
, blockConstructor = fmap mconcat . renderChildren
, blockFinalize = \(Node cdata children) parent -> do
Expand Down
89 changes: 89 additions & 0 deletions commonmark/test/regression.md
Original file line number Diff line number Diff line change
Expand Up @@ -361,3 +361,92 @@ Issue #149
<p><a href="!">link</a></p>
````````````````````````````````

Issue #144
```````````````````````````````` example
+ Is this wrapping list tight, or loose?
* This nested list is definitely tight.
-
-
.
<ul>
<li>Is this wrapping list tight, or loose?
<ul>
<li>This nested list is definitely tight.</li>
</ul>
<ul>
<li></li>
<li></li>
</ul>
</li>
</ul>
````````````````````````````````
```````````````````````````````` example
+ Is this wrapping list tight, or loose?
* This nested list is definitely tight.
- First item
-
.
<ul>
<li>Is this wrapping list tight, or loose?
<ul>
<li>This nested list is definitely tight.</li>
</ul>
<ul>
<li>
<p>First item</p>
</li>
<li></li>
</ul>
</li>
</ul>
````````````````````````````````
```````````````````````````````` example
+ Is this wrapping list tight, or loose?
* This nested list is definitely tight.
-
- Second item
.
<ul>
<li>Is this wrapping list tight, or loose?
<ul>
<li>This nested list is definitely tight.</li>
</ul>
<ul>
<li></li>
<li>
<p>Second item</p>
</li>
</ul>
</li>
</ul>
````````````````````````````````
```````````````````````````````` example
+ Is this wrapping list tight, or loose?
* This nested list is definitely tight.
- First item
- Second item
.
<ul>
<li>Is this wrapping list tight, or loose?
<ul>
<li>This nested list is definitely tight.</li>
</ul>
<ul>
<li>
<p>First item</p>
</li>
<li>
<p>Second item</p>
</li>
</ul>
</li>
</ul>
````````````````````````````````
Expand Down

0 comments on commit 1ec2293

Please sign in to comment.