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

feat: Generate table of contents code action #17

Merged
merged 11 commits into from
Jun 9, 2022

Conversation

keynmol
Copy link
Contributor

@keynmol keynmol commented Jun 8, 2022

Very basic demonstration of how a "Create TOC" can be implemented.
Note that I don't know F# at all :D

TODO:

  1. Learn F#

  2. Detect an existing TOC and update it when the code action is invoked (can even put diagnostics on the exact locations saying it's out of date)

    This is actually non-trivial if done in a non-invasive manner - only heuristic that can possibly exist is "first block of nothing but links in the document" and it can be easily broken. Can also match those links to a set of headings for some overlap number.

    2022-06-08 21 56 50

Marksman/Server.fs Outdated Show resolved Hide resolved
Marksman/Server.fs Outdated Show resolved Hide resolved
Marksman/Server.fs Outdated Show resolved Hide resolved
Marksman/Server.fs Outdated Show resolved Hide resolved
@artempyanykh
Copy link
Owner

Detect an existing TOC and update it when the code action is invoked (can even put diagnostics on the exact locations saying it's out of date)

This is actually non-trivial if done in a non-invasive manner - only heuristic that can possibly exist is "first block of nothing but links in the document" and it can be easily broken. Can also match those links to a set of headings for some overlap number.

@keynmol what do you think about generating toc with a heading ## Table of Contents (or with a slug of the heading being table-of-contents and then consider this heading and everything under it as a toc?

@artempyanykh
Copy link
Owner

Cool stuff!
giphy

@keynmol
Copy link
Contributor Author

keynmol commented Jun 8, 2022

New version, capable of updating the existing TOC

2022-06-08 21 56 50

@keynmol keynmol marked this pull request as ready for review June 8, 2022 20:59
Marksman/Toc.fs Outdated Show resolved Hide resolved
@keynmol keynmol changed the title Add a super shoddy POC for table of contents feat: Generate table of contents code action Jun 9, 2022
@keynmol
Copy link
Contributor Author

keynmol commented Jun 9, 2022

One issue with this is that currently Marksman's parser detects YAML frontmatter stuff like:

---
title: Hello
---

As a heading, and adds it to the TOC. I believe this is parser's deficiency, as it interprets the yaml block (which arguably isn't a part of the markdown spec) as one of the syntaxes for headings:

image

What makes matters worse is that --- is a horizontal line (https://spec.commonmark.org/0.30/#example-43), so you can't even technically parse the entire yaml frontmatter away :)

Markdown is hard

Only heuristic is really that the first line of the file is --- and there's a closing one as well, and the user somehow indicated that they want yaml frontmatter detection enabled

Marksman/Server.fs Outdated Show resolved Hide resolved
Marksman/Server.fs Outdated Show resolved Hide resolved
Marksman/Server.fs Outdated Show resolved Hide resolved
Marksman/Toc.fs Outdated Show resolved Hide resolved
Marksman/Toc.fs Outdated Show resolved Hide resolved
Marksman/Toc.fs Outdated Show resolved Hide resolved
Tests/Helpers.fs Outdated Show resolved Hide resolved
Tests/TocTests.fs Outdated Show resolved Hide resolved
@artempyanykh
Copy link
Owner

artempyanykh commented Jun 9, 2022

One issue with this is that currently Marksman's parser detects YAML frontmatter stuff like:

@keynmol I think markdig (the lib we use) has some support for yaml front matter -- you may want to look into mark dig pipeline in Parser.fs, perhaps there's some extension that needs to be activated to fix front matter handling.

UPDATE: we can do it in a separate PR if you want. I'd rather merge this one and improve separately, so that GH stop considering you a "First time contributor" and making me approve running CI on your PRs 😄

@keynmol
Copy link
Contributor Author

keynmol commented Jun 9, 2022

Ok, game plan:

  1. Address the nits
  2. Change the end marker to be another comment (helps with counting these damn newlines as well)
  3. Merge
  4. Enable yaml frontmatter and add tests in a separate PR

@artempyanykh artempyanykh merged commit 17d61ae into artempyanykh:main Jun 9, 2022
@keynmol keynmol deleted the table-of-contents branch June 9, 2022 12:47
@artempyanykh
Copy link
Owner

Awesome! 🎉 Thanks for the contribution ❤️

This pull request was closed.
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