Skip to content

fix: correct markdown header processing - #539

Merged
jcreedcmu merged 25 commits into
leanprover:mainfrom
jcreedcmu:jcreed-markdown-fix
Oct 1, 2025
Merged

fix: correct markdown header processing#539
jcreedcmu merged 25 commits into
leanprover:mainfrom
jcreedcmu:jcreed-markdown-fix

Conversation

@jcreedcmu

Copy link
Copy Markdown
Collaborator

Previously, parts generated by addPartFromMarkdown were at wrong nesting depth for nontrivial structures. Error explanations (for which this code was originally written) did not exercise this but because they have fairly simple structure, without many children of the same header, or decreases of header depth.

This PR simplifies the verso-nesting-depth-to-markdown-nesting-depth state, to a list of Nat rather than a list of pair of Nats. I think without sacrificing correctness.

@jcreedcmu

Copy link
Copy Markdown
Collaborator Author

I added some incidental comments to some Verso code as well based on my current understanding of it, let me know if I said anything wrong.

jcreedcmu added a commit to jcreedcmu/reference-manual that referenced this pull request Sep 29, 2025
Comment thread src/verso-manual/VersoManual/Markdown.lean Outdated
Comment thread src/verso-manual/VersoManual/Markdown.lean Outdated
Co-authored-by: David Thrane Christiansen <david@davidchristiansen.dk>
Comment thread src/verso-manual/VersoManual/Markdown.lean Outdated
jcreedcmu and others added 2 commits October 1, 2025 10:27
Co-authored-by: David Thrane Christiansen <david@davidchristiansen.dk>
Co-authored-by: David Thrane Christiansen <david@davidchristiansen.dk>
Comment thread src/verso-manual/VersoManual/Markdown.lean
Comment thread src/verso-manual/VersoManual/Markdown.lean
Comment thread src/verso-manual/VersoManual/Markdown.lean Outdated
Comment thread src/verso-manual/VersoManual/Markdown.lean Outdated
Comment thread src/verso-manual/VersoManual/Markdown.lean Outdated
Comment thread src/verso-manual/VersoManual/Markdown.lean Outdated
Comment thread src/verso-manual/VersoManual/Markdown.lean Outdated
jcreedcmu and others added 3 commits October 1, 2025 10:35
Co-authored-by: David Thrane Christiansen <david@davidchristiansen.dk>
Co-authored-by: David Thrane Christiansen <david@davidchristiansen.dk>
Co-authored-by: David Thrane Christiansen <david@davidchristiansen.dk>
Comment thread src/verso-manual/VersoManual/Markdown.lean Outdated
Co-authored-by: David Thrane Christiansen <david@davidchristiansen.dk>
Comment thread src/verso-manual/VersoManual/Markdown.lean Outdated
Comment thread src/verso-manual/VersoManual/Markdown.lean Outdated
Comment thread src/verso-manual/VersoManual/Markdown.lean Outdated
Comment thread src/verso-manual/VersoManual/Markdown.lean
Comment thread src/verso/Verso/Doc/Elab/Monad.lean
Comment thread src/verso/Verso/Doc/Elab/Monad.lean Outdated
Comment thread src/verso/Verso/Doc/Elab/Monad.lean Outdated
Comment thread src/verso/Verso/Doc/Elab/Monad.lean Outdated
jcreedcmu and others added 2 commits October 1, 2025 10:51
Co-authored-by: David Thrane Christiansen <david@davidchristiansen.dk>
Co-authored-by: David Thrane Christiansen <david@davidchristiansen.dk>
Comment thread src/verso/Verso/Doc/Elab/Monad.lean Outdated
jcreedcmu and others added 2 commits October 1, 2025 10:55
Co-authored-by: David Thrane Christiansen <david@davidchristiansen.dk>
Co-authored-by: David Thrane Christiansen <david@davidchristiansen.dk>
@david-christiansen

Copy link
Copy Markdown
Collaborator

Apart from nitpicky docstring stuff, I think this is a nice simplification and I think the docs were all correct.

Did you have set_option doc.verso true in the file? If not, then Verso docstrings probably shouldn't be used quite yet :)

@david-christiansen

Copy link
Copy Markdown
Collaborator

(or, alternatively, make sure to enable it)

jcreedcmu and others added 8 commits October 1, 2025 10:58
Co-authored-by: David Thrane Christiansen <david@davidchristiansen.dk>
Co-authored-by: David Thrane Christiansen <david@davidchristiansen.dk>
Co-authored-by: David Thrane Christiansen <david@davidchristiansen.dk>
Co-authored-by: David Thrane Christiansen <david@davidchristiansen.dk>
jcreedcmu added a commit to jcreedcmu/reference-manual that referenced this pull request Oct 1, 2025
@jcreedcmu
jcreedcmu merged commit 293d416 into leanprover:main Oct 1, 2025
4 checks passed
@jcreedcmu
jcreedcmu deleted the jcreed-markdown-fix branch October 1, 2025 15:53
jcreedcmu added a commit to jcreedcmu/verso that referenced this pull request Oct 1, 2025
Make the type `HeaderMapping` an abbreviation instead of a definition.

This change was intended as part of
leanprover#539 and will make
leanprover/reference-manual#599 easier to
land. There's still [one place
remaining](https://github.com/leanprover/reference-manual/blob/main/Manual/Meta/ErrorExplanation.lean#L639-L640)
in `Manual/Meta/ErrorExplanation.lean` where it's relying on the
implementation of this type, which breaks if typeclass resolution
doesn't see the `ForIn` instance on `List`. Of course, we could also
expose `ForIn` on `HeaderMapping`, or expose a "close all sections for
this `HeaderMapping` but I'd rather decouple that decision from [the
present PR](leanprover/reference-manual#599).
jcreedcmu added a commit that referenced this pull request Oct 1, 2025
Make the type `HeaderMapping` an abbreviation instead of a definition.

This change was intended as part of
#539 and will make
leanprover/reference-manual#599 easier to
land. There's still [one place
remaining](https://github.com/leanprover/reference-manual/blob/main/Manual/Meta/ErrorExplanation.lean#L639-L640)
in `Manual/Meta/ErrorExplanation.lean` where it's relying on the
implementation of this type, which breaks if typeclass resolution
doesn't see the `ForIn` instance on `List`. Of course, we could also
expose `ForIn` on `HeaderMapping`, or expose a "close all sections for
this `HeaderMapping` but I'd rather decouple that decision from [the
present PR](leanprover/reference-manual#599).
jcreedcmu added a commit to jcreedcmu/verso that referenced this pull request Oct 1, 2025
Previously, parts generated by `addPartFromMarkdown` were at wrong nesting depth for nontrivial structures. Error explanations (for which this code was originally written) did not exercise this but because they have fairly simple structure, without many children of the same header, or decreases of header depth.

This PR simplifies the verso-nesting-depth-to-markdown-nesting-depth state, to a list of `Nat` rather than a list of pair of `Nat`s. I think without sacrificing correctness.
jcreedcmu added a commit to jcreedcmu/verso that referenced this pull request Oct 1, 2025
Make the type `HeaderMapping` an abbreviation instead of a definition.

This change was intended as part of
leanprover#539 and will make
leanprover/reference-manual#599 easier to
land. There's still [one place
remaining](https://github.com/leanprover/reference-manual/blob/main/Manual/Meta/ErrorExplanation.lean#L639-L640)
in `Manual/Meta/ErrorExplanation.lean` where it's relying on the
implementation of this type, which breaks if typeclass resolution
doesn't see the `ForIn` instance on `List`. Of course, we could also
expose `ForIn` on `HeaderMapping`, or expose a "close all sections for
this `HeaderMapping` but I'd rather decouple that decision from [the
present PR](leanprover/reference-manual#599).
@jcreedcmu

Copy link
Copy Markdown
Collaborator Author

Verso docstrings were later enabled in #547

jcreedcmu added a commit to jcreedcmu/reference-manual that referenced this pull request Oct 8, 2025
jcreedcmu added a commit to jcreedcmu/reference-manual that referenced this pull request Oct 8, 2025
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 this pull request may close these issues.

2 participants