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

[fuzz result] unindented lines after footnote def are silently eaten #126

Closed
notriddle opened this issue Oct 31, 2023 · 6 comments
Closed

Comments

@notriddle
Copy link
Contributor

Consider this markdown:

[^foo]:bar
baz

[^foo]

It's debatable whether baz should be part of the footnote or not, but it should certainly appear in the output somewhere. Right now, commonmark-extensions discards it.

In GitHub, it renders like this:

1

Footnotes

  1. bar
    baz

@jgm
Copy link
Owner

jgm commented Nov 1, 2023

Yes, that should count as a lazy continuation.

@jgm
Copy link
Owner

jgm commented Nov 1, 2023

I don't understand why the lazy line isn't being handled properly. Look at blockQuoteSpec for an analogy. This one should behave the same!

@jgm
Copy link
Owner

jgm commented Nov 1, 2023

The disanalogy is actually in blockFinalize.
I wonder if that's the clue.

@jgm
Copy link
Owner

jgm commented Nov 1, 2023

Evidence for this: if you change blockFinalize for footnote to defaultFinalizer, then the lazy line appears in a regular paragraph.

@jgm
Copy link
Owner

jgm commented Nov 1, 2023

OK, I think I understand now what is happening.

In processLine, we close unmatched blocks
https://github.com/jgm/commonmark-hs/blob/master/commonmark/src/Commonmark/Blocks.hs#L138-L147
and in this case that would mean we close the footnote when we hit the unindented line. At this point the note's paragraph just contains "bar".

Laziness is handled at
https://github.com/jgm/commonmark-hs/blob/master/commonmark/src/Commonmark/Blocks.hs#L154-L159
Essentially, when we hit a lazy continuation, we simply add the closed blocks back to the node stack and proceed as before.

Normally, this works fine! But in the case of footnoteBlockSpec, it doesn't, because in this case blockFinalize has a side effect -- it updates the reference map with the note contents.

In fact, the reference map will be updated again after the re-added footnote is closed again (this time with "baz"). So at this point there will be two entries with key "foo." But lookupReference is set up to prefer the first entry in case of duplicates, so we get the defective one.

@jgm
Copy link
Owner

jgm commented Nov 1, 2023

Solution? I'm not sure. It does seem non-ideal that in our treatment of lazy lines, we end up calling blockFinalize twice (or even more) for the same block, even though in most cases this is harmless. So, perhaps, instead of closing the blocks at line 138, we should wait until we actually detect a new block -- at that point we'd need to close the unmatched ones. If no new block is detected, then, unless we had a lazy line, we could close unmatched blocks, but if we had a lazy line, we'd still have the unmatched blocks open and wouldn't need to re-add them. This might even improve performance a bit when there are lots of lazy lines.

@jgm jgm closed this as completed in 6ec393d Nov 2, 2023
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 a pull request may close this issue.

2 participants