-
-
Notifications
You must be signed in to change notification settings - Fork 110
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
WIP - Add table of content option and generation #75
base: master
Are you sure you want to change the base?
Conversation
You can follow the progression . Should I provide a default theme for the content table or let the user provide his own? |
I just test my implementation with page break. All headings after page break are not take into account. Any idea to fix this? |
Ok, this is corrected. Just one test that failed.
I don't understand how this can be validated as getHtml always return string with tag. any idea? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just while you're at it, I'm not really a fan of OOP, so can you change the InitMarked
class back to a function instead? It would be nice if the TOC generation function is a separate module and we can load it with marked.use()
.
Another thing we need to do is check !Object.prototype.hasOwnProperty.call(renderer, 'heading')
because we don't want to override a user's custom heading renderer.
Also, I find the current configuration too complicated. It should just check sth like
html.includes('<!-- TOC -->')
and if present, it should automatically be enabled. Then toc_heading
and toc_depth
can be two new config options (not nested).
Alternatively, we could actually go with <div class="toc"></div>
because then we could allow custom content there, like
<div class="toc">
## Table of Contents
</div>
(the blank lines are important for this mix of html and markdown btw)
and then append the TOC. More complicated to do this though (as we can't use document.querySelector
, unless we only append the TOC in the browser context).
Ok i will update ma PR soon. The type definition for Marked is out of date and missing use function. |
Cool thanks! You can do |
I have made a PR on definitlytyped to update the typings. |
a9fe4fc
to
61cb8fd
Compare
Ok i have make the change. but there is still the test 'getHtml should inject rendered markdown' that does not pass |
@@ -1,20 +1,18 @@ | |||
import { getLanguage, highlight } from 'highlight.js'; | |||
import marked, { MarkedOptions } from 'marked'; | |||
|
|||
export const getMarked = (options: MarkedOptions) => { | |||
export const getHighlightRenderer = (options: MarkedOptions) => { | |||
const renderer = options.renderer ?? new marked.Renderer(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't make sense to have this in combination with marked.use()
anymore. If there's options.renderer
, then we need to call marked.use({ renderer: options.renderer })
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you explain me a bit more ? i'm not see what modification i need to do.
export const getHeadingRenderer = (config: Config) => { | ||
toc = []; | ||
const options = config.marked_options; | ||
const renderer = options.renderer ?? new marked.Renderer(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing as previous comment, we don't want to extend the user's custom renderer.
Great news. My PR for the type was accepted . DefinitelyTyped/DefinitelyTyped#45578 |
Hey @scandinave I've gotten busy with other work and won't be able to have a proper look at this in the next couple weeks. Are you ok to just use your fork in the mean-time? You can I'll get to it when I have some spare time for this project again. |
Ok no problem. Take your time :) |
Hello, I wonder what will happen for people using an extension to manage their TOC. Like Markdown TOC in VSCode. <!-- TOC -->
- [Title 1](#title-1)
- [Title 1.1](#title-11)
- [Title 2](#title-2)
- [Title 2.2](#title-22)
<!-- /TOC -->
# Title 1
## Title 1.1
# Title 2
# Title 2.2 I suppose you are using If that's the case, I think it would be nice to add a new options to control if TOC should be generated or not and use |
Could just use |
I can just add a check for |
Any news on the TOC feature ? |
PR for #74