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

add the slugified URL to the AST #46

Open
rdeltour opened this issue Dec 9, 2020 · 4 comments
Open

add the slugified URL to the AST #46

rdeltour opened this issue Dec 9, 2020 · 4 comments
Assignees
Labels
question Further information is requested

Comments

@rdeltour
Copy link

rdeltour commented Dec 9, 2020

it would be convenient if the generated AST had a field to hold the slugified links. I know we can augment the AST using the callback function, but having it built-in ensures the same slugify method is used.

Example of current AST:

{
  "l": 2,
  "n": "My Title",
  "c": []
}

Example of proposed AST:

{
 "l": 2,
 "n": "My Title",
 "u": "#my-title",
 "c": []
}

I can make a PR if you’re interested!

@nagaozen nagaozen self-assigned this Dec 23, 2020
@nagaozen
Copy link
Owner

I believe this is somehow connected to #47. Same idea applies, why not slugify by yourself? Decoupling the AST from the slugifyer seems to be a better architecture.

@nagaozen nagaozen added the question Further information is requested label Dec 23, 2020
@rdeltour
Copy link
Author

why not slugify by yourself? Decoupling the AST from the slugifyer seems to be a better architecture.

Yeah, I agree that with the current approach it is not critical and it is rather easy to do it in the callback.

But in fact, I'm even wondering if in most cases the heading id could not be retrieved from the heading token itself (when building the AST) rather than from slugifying its content?

For instance, what if the id was added by something else than markdown-it-anchor, like manually added by markdown-it-attrs. The slugify function would not return the right ID in that case…

Is the ID available from the token at the time the AST is built?

@nagaozen
Copy link
Owner

nagaozen commented Dec 24, 2020

Oh, thank you! Just made a test and confirm that you are right. Using markdown-it-attrs to define custom id's makes the AST not useful:

let md = require('markdown-it')();
let markdownItAttrs = require('markdown-it-attrs');
let markdownItAnchor = require('markdown-it-anchor');
let markdownInToc = require('markdown-it-toc-done-right')

md.use(markdownItAttrs)
  .use(markdownItAnchor)
  .use(markdownInToc);

let src = [
'[TOC]',
'# header {#custom_id}',
'paragraph {data-toggle=modal}'
].join('\n')

md.render(src);

So yeah, we probably need to put the output id in the AST as you pointed out. using "u" as unique abbreviation is fine for me.

@shellscape
Copy link

FWIW we're using this to capture the TOC AST for rendering the TOC in a separate page component. An updated AST would be extremely useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants