cmaas / markdown-it-table-of-contents

A table of contents plugin for Markdown-it
MIT License
99 stars 33 forks source link

Does not parse emphesis like **bold** or *italic* #19

Closed cougarten closed 3 years ago

cougarten commented 7 years ago

example: # My **bold** title

should render as: <li>My <strong>bold</strong> title</li>

does render as: <li>My **bold** title</li>

martinlissmyr commented 7 years ago

Hi. A PR with a fix would be very welcome. Although I would argue that no styling should be transfered to the TOC. The formatting should rather be stripped away.

char0n commented 6 years ago

format hook can be used to render this markup as html. But this creates additional problem how to get only text nodes from this rendered html...

char0n commented 6 years ago

Workaround:

const removeMarkdown = require('remove-markdown');
const toc = require('markdown-it-table-of-contents');
const md = require('markdown-it')();

md.use(toc, {
  format: removeMarkdown,  
});

const src = `
 [[toc]]
 # *Title*
 ## Subtitle 
`;

md.render(src, {});
char0n commented 6 years ago

I guess we can close this. Workaround/Solution mentioned above work for most usecases.

martinlissmyr commented 6 years ago

👍

coryschires commented 6 years ago

Hi. Thanks for making this plugin! It's been really helpful.

I'd like to re-raise this issue. By default, I think this library should either:

  1. Strip out markdown (e.g. My **bold** title becomes My bold title).
  2. Process the markdown (e.g. My **bold** title becomes My <strong>bold</strong> title)

The current default behavior – rendering unprocessed markdown – is probably not what anyone wants or expects. I understand you can achieve either of these outcomes using a format function. But, ideally, users of this extension could avoid that work if a smarter default were in place.

Of the two options I've suggested, I think it would be better and easier to process the markdown. (Stripping could be tricky given the full range of possibilities, especially if you consider rarer options like superscript, subscript, etc.).

I'm using this lib to render ToCs for scientific articles. In my (admittedly complex) use case, headers may include a variety of formatting (e.g. italics, superscripts, even equations). And this formatting is often integral to header's legibility / meaning (i.e. it's not just a matter of style).

How to implement / Need help?

Looking at your code, I think the md object may have access to the underlying rendering functions. For example, I wrote a markdown-it extension which did something like:

this.env.md.renderInline("Some text which _may have formatting_")

And this allowed me to process markdown text within my extension – which I think is what we want in this case as well.

I'm not 100% sure how this would work in your code, but I think it's possible (and probably pretty easy). If you need help, lemme know and could probably make a PR.

martinlissmyr commented 6 years ago

Hi! Thanks for chipping in...

The current default behavior – rendering unprocessed markdown – is probably not what anyone wants or expects.

I agree.

I'm using this lib to render ToCs for scientific articles. In my (admittedly complex) use case, headers may include a variety of formatting (e.g. italics, superscripts, even equations). And this formatting is often integral to header's legibility / meaning (i.e. it's not just a matter of style).

I haven't considered this scenario. It makes sense.

Of the two options I've suggested, I think it would be better and easier to process the markdown.

Given your examples I see your point. However, I think it would be preferable if it could be an option like so: parseMarkdownInHeadings: true or false where true should be the default option.

Do you think it's somewhat plausible to implement stripping markdown? Haven't looked at it in any depth but I imagine it's possible by either hooking into the markdown-it parser or by stripping markdown in the raw text via regexp or similar (I've seen libs that do that)...

coryschires commented 6 years ago

Thanks for considering this change! I like the idea of adding a parseMarkdownInHeadings option which defaults to true.

As I said, I think processing the markdown should be easy (maybe even a one-liner). Stripping the markdown is a bit trickier.

Here's what I would suggest:

1. Use an existing library

Both remove-markdown and strip-markdown seem decent. That said, they do seem a little greedy and could result in striping non-markdown text in rare edge cases (e.g. if your heading includes a non-markdown _ or [). But maybe that's okay?

2. Don't even try

Maybe, instead, give folks the option to render the raw markdown (i.e. parseMarkdownInHeadings: false) and let them deal with the complexity as well as the potential edge cases by writing a custom format function. For example, if I know my headers will only include *, then a simple regex would suffice. Or, if I have more possibilities, then I could consider using something like remove-markdown within my format function.

Most of all, I would avoid trying to write your own regex. Just my opinion but I think it could end up being a messy, half-solution. And, given a tricky problem, I'd rather avoid it altogether than apply a partial fix. But that's just my opinion.

martinlissmyr commented 6 years ago

Ok, I hear you...

Then maybe we could just consider changing the default for the format option to a function that returns parsed markdown. Sort of what you initially suggested 😄 ...

A PR with this change would be very welcome!

coryschires commented 5 years ago

@martinlissmyr Sorry I dropped the ball on this fix for several months. But, finally, I have a PR for you to review: https://github.com/Oktavilla/markdown-it-table-of-contents/pull/41

Thanks again for creating this extension!