`gfmExtensions` parser drops out of list when `|` is encountered
andreasabel opened this issue · 15 comments
( Lifted from
Rendering
- foo
- `a|b`
- bar
with gfmExtensions
included in the parser gives this:
- foo
- a|b
- bar
- foo
a|b
- bar
Reproducer:
{-# LANGUAGE LambdaCase #-}
{-# LANGUAGE QuasiQuotes #-}
{-# LANGUAGE ScopedTypeVariables #-}
import Commonmark
import Commonmark.Extensions
import Data.String.QQ (s)
import Data.Text.Lazy.IO as TLIO
main :: IO ()
main = do
commonmarkWith (gfmExtensions <> defaultSyntaxSpec) "inline" input >>= \case
Left e -> error (show e)
Right (html :: Html ()) -> TLIO.putStr $ renderHtml html
where
input = [s|
- foo
- `a|b`
- bar
|]
{- Produces:
<ul>
<li>foo
</li>
</ul>
<p>- <code>a|b</code></p>
<ul>
<li>bar
</li>
</ul>
Seems like the gfmExtensions parser cannot deal with `|`.
-}
I think what's happening is that the table extension is kicking in, because of the |
.
I think the current behavior of this extension is to regress and parse the lines as paragraph content if a table heading line doesn't follow -- obviously, this isn't what is wanted in the current case. In fact, this case may interact badly with the commonmark parser's goal of parsing line by line with no backtracking.
The relevant part of the source is in commonmark-extensions, src/Commonmark/Extensions/PipeTable.hs, lines 176-193.
You may be able to work around this by moving the pipeTableSpec
after defaultSyntaxSpec
, i.e.
allTheGfmExtensionsExceptPipeTable <> defaultSyntaxSpec <> pipeTableSpec
I think what's happening is that the table extension is kicking in, because of the
|
.
I think the current behavior of this extension is to regress and parse the lines as paragraph content if a table heading line doesn't follow
commonmark-hs/commonmark-extensions/src/Commonmark/Extensions/PipeTable.hs
Lines 183 to 195 in 714a1b6
Judging from the examples given with https://github.github.com/gfm/#table, and its present implementation on github.com, a table header needs to be followed by a delimiter row, otherwise it is not a table.
i|am|not|a|table
|i|am|also|not|a|table|
i | am | a | table |
---|
i | am | also | a | table |
---|
In fact, this case may interact badly with the commonmark parser's goal of parsing line by line with no backtracking.
Apparently, you need two lines to start a table. So yes, noble goal, but reality is harsh... ;-)
You may be able to work around this by moving the
pipeTableSpec
afterdefaultSyntaxSpec
, i.e.allTheGfmExtensionsExceptPipeTable <> defaultSyntaxSpec <> pipeTableSpec
Thanks for the hint!
I am a bit hesitant to follow this path, because I cannot judge the consequences. How do I know this isn't a whack-a-mole-game? We haven't secured desired behaviors of our markdown parser instance with a testsuite, so I might break something else.
I'd rather wait for a fix of the table parser...
a table header needs to be followed by a delimiter row, otherwise it is not a table.
Yes, and that's what this parser expects. The difficulty is that this parser tries to do line-by-line block parsing with no backtracking, so it can't look ahead to the next line. The current approach is to retroactively change the block from a table to a paragraph if that second line isn't encountered. But that won't work for the kind of example you give. The fix, with the current parsing strategy, isn't obvious, unless it's the thing I mention above. Note that the monoidal composition of syntax specs is order-sensitive. So the problem here is that we're checking for a table before checking for a list item.
Ok, I try the workaround then!
The problem with the suggested workaround (pipeTableSpec
after defaultSyntaxSpec
) is that tables do not parse anymore:
{-# LANGUAGE LambdaCase #-}
{-# LANGUAGE OverloadedStrings #-}
{-# LANGUAGE ScopedTypeVariables #-}
import Commonmark
import Commonmark.Extensions
import Data.Text.Lazy.IO as TLIO
main :: IO ()
main = do
commonmarkWith spec "inline" input >>= \case
Left e -> error (show e)
Right (html :: Html ()) -> TLIO.putStr $ renderHtml html
where
spec = mconcat
[ emojiSpec
, strikethroughSpec
, autolinkSpec
, autoIdentifiersSpec
, taskListSpec
, footnoteSpec
, defaultSyntaxSpec
, pipeTableSpec
]
input = table
table = "|a|table|\n|---|---|"
Gives:
<p>|a|table|
|---|---|</p>
OK, back to the drawing board.
Here's another case to keep in mind:
- `a|b`
- | -
This does parse -- as a table. Hm, I wonder how GitHub's parser renders it? Let's try:
a|b
- | -
Let's also try your original case:
- foo
a|b
- bar
Literal pipes in GitHub's pipe tables formerly needed to be escaped, even inside code backticks; let's see if that's still the case:
test | table |
---|---|
` | ` |
Answer: yes.
Aha. You need a trailing newline in input
in your program above. That's why it's not parsing as a table.
Original case also works with the workaround; you just need to ensure that all lines are terminated with newline characters.
I don't think the workaround will have any bad consequences: it just means that nothing will be interpreted as a table if it can be interpreted as any other kind of block-level element (aside from a paragraph), and I think that's desirable.
We can leave this issue open, though, because this is an awkward workaround and it makes it impossible to use gfmExtensions
.
Thanks for the further research, @jgm. I now applied your workaround to hackage-server
: