medfreeman / markdown-it-toc-and-anchor

markdown-it plugin to add a toc and anchor links in headings
MIT License
60 stars 35 forks source link

link inside heading not rendered correctly. #7

Closed tylerlong closed 8 years ago

tylerlong commented 8 years ago

Example:

# [test](http://google.com)

## Table of Contents

@[toc]

Rendered:

screen shot 2016-01-09 at 10 44 25 pm

We can see that the raw markdown it shown in TOC.

Similarly, # **text**, # *text*, # ~~text~~ are not rendered correctly in TOC.

And emojis has this problem too: # :dog:

I think the key to the problem is: After this plugin generate TOC, the content of TOC should still go through processing of other plugins such as emoji.

I tried to solve it, but hasn't made much progress yet. It is appreciate if some one could give me some hints. I would be more than happy to get this done and submit a PR.

Thank you in advance.

MoOx commented 8 years ago

That's an issue indeed. I am not really good with markdown-it either, that why I didn't fix that yet :/

medfreeman commented 8 years ago

That's because the TOC function is in renderer rules, i think it should be in core rules, so you first generate the toc in markdown syntax, then let markdown-it convert it while going through block and inline rules. I'll start to implement this now.

MoOx commented 8 years ago

Thanks for your contribution.

medfreeman commented 8 years ago

Thanks for your project, it is extremely valuable ! The work is done here : https://github.com/medfreeman/markdown-it-toc-and-anchor/tree/wip

However there's a few changes that i wanted to discuss.

First, i removed the 'indentation' option, i'll explain why. Let me highlight the way things are done in my fork first, in a core markdown-it rule :

Since now the toc in markdown format is parsed by markdown-it itself, it renders the toc without indentation. Should i re-implement this feature by altering markdown-it html rendering rules for the toc ?

Second and last, the test for an empty toc does currently return <p></p> instead of also containing the empty toc 'ul' element. That's because an empty headings markdown would not generate anything except the encasing 'p' tags.

Would you prefer that i reimplement these behaviors before making a pull request ?

Thx

medfreeman commented 8 years ago

I knew i forgot something ! For now an heading containing a link generates a toc list element containing a link to the corresponding heading anchor, which in its turn links to the original url. I believe it is the correct behavior, but it could also be configurable. As to also allow the toc list element to link directly to the url, without jumping to the corresponding anchor.

MoOx commented 8 years ago

@medfreeman just to let you know, I don't use markdown-it anymore. I use remark now. So adding you as a collaborator as I don't have time to maintain this package anymore. Just 2 things: please respect semver (reach me if you are not aware of this, maybe checking the changelog and version number will help you: breaking change: major; new feature: minor; bgfix: patch).

In order to make a release, you have to do 2 things:

MoOx commented 8 years ago

send me your npm nickname :)

medfreeman commented 8 years ago

Thank you for your consideration ! I'll respect these guidelines. My npm's med_freeman. If you got a moment to comment on the issues i mentioned here, i'll be happy to know your opinion, since i'm pretty new to markdown-it / commonmark, but no obligations. Thanks !

MoOx commented 8 years ago
❯ npm owner add med_freeman
+ med_freeman (markdown-it-toc-and-anchor)

I was new to markdown-it too. But I found authoring plugin for markdown-it is too complicated. I prefer remark/mdast approach.

medfreeman commented 8 years ago

Hi, sorry for all the notifications you probably have received about tags and releases. It was the first time for me, and i think i now understand the whole process correctly. I'll do this in one go next time.

MoOx commented 8 years ago

Not that you made a release with internal stuff (eslint and ava), which is not required at all :) (no need to add this in the CHANGELOG when it's not part of public API)

MoOx commented 8 years ago

Also please next time, use a commit for bumping version number, make commit log cleaner (and more explicit).

MoOx commented 8 years ago

Also, don't need to use your fork. You have write access here :)

medfreeman commented 8 years ago

Thx, not sure about the fork part, since i tend to make mistakes using git correctly, using mine allows to do all kinds of take-backs. I'll stick to mine till i better understand all the quirks.