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

toc helper should not parse the text of children element(s) #174

Closed
2 tasks done
noraj opened this issue Jan 5, 2020 · 10 comments · Fixed by #175
Closed
2 tasks done

toc helper should not parse the text of children element(s) #174

noraj opened this issue Jan 5, 2020 · 10 comments · Fixed by #175

Comments

@noraj
Copy link

noraj commented Jan 5, 2020

Check List

Please check followings before submitting a new feature request.

  • I have already read Docs page
  • I have already searched existing issues

Feature Request

Fix the toc helper to select only text content and not other sub-elements like links.

In depth explanation here: hexojs/hexo-renderer-markdown-it#102

@SukkaW
Copy link
Member

SukkaW commented Jan 5, 2020

@noraj

Fix the toc helper to select only text content and not other sub-elements like links.

Is this issue show up in Hexo 4.1.1? I am wondering if the problem is the different behavior between htmlparser2 & cheerio.

@noraj
Copy link
Author

noraj commented Jan 5, 2020

@curbengh curbengh changed the title Fix the toc helper to select only text content and not other sub-elements like links. toc helper should not parse the text of children elements Jan 10, 2020
@curbengh curbengh changed the title toc helper should not parse the text of children elements toc helper should not parse the text of children element(s) Jan 10, 2020
@curbengh
Copy link
Contributor

curbengh commented Jan 10, 2020

Can confirm the issue in hexo-util 1.8.1:

const { tocObj } = require('hexo-util')

const text = '<h3 id="create-a-new-post"><a class="header-anchor" href="#create-a-new-post">#</a>Create a new post</h3>'

console.log(tocObj(text))
/*
[
  {
    text: '#Create a new post',
    id: 'create-a-new-post',
    level: 3
  }
]
*/

@curbengh curbengh transferred this issue from hexojs/hexo Jan 10, 2020
@SukkaW
Copy link
Member

SukkaW commented Jan 10, 2020

@curbengh

<h1 id="title-3"><span>Title</span> 3</h3>

In this case Title 3 is what we needed for text.

@curbengh
Copy link
Contributor

curbengh commented Jan 10, 2020

@SukkaW

Since the output is '\n#\nCreate a new post\n', what if tocObj trim the newlines and anything before and after \n? It may be more appropriate to trim in toc helper instead, since tocObj user might want to retain the original (rare, but possible).

Edit: no newline by default

@SukkaW
Copy link
Member

SukkaW commented Jan 10, 2020

@curbengh

Currently,

<h3 id="create-a-new-post">
<a class="header-anchor" href="#create-a-new-post">#</a>Create a new post
</h3>

will become \n#Create a new post\n. After trimming it will become Create a pos. Still not appropriate.


Update

cheerio has the same behavior.

https://runkit.com/sukkaw/5e186c9d286975001a1051c2

@curbengh
Copy link
Contributor

Looks like a common question in cheerio/jquery,

Seems possible to fix, if htmlparser has the equivalent function of native DOM's document.childNodes or cheerio's $.first().

@Microbai
Copy link

When contents likes this

<h2><span id="title_1">title 1</span></h2>

tocObj won't parse the correct id, So I edit the getId function, then work.

const getId = ele => {
  const { id } = ele.attribs;
  const { children } = ele;
  return id || (!children[0] ? null : getId(children[0]));
};

@SukkaW
Copy link
Member

SukkaW commented Jan 30, 2020

@Microbai tocObj() is not designed to get id from child element but only from parent element.

@Microbai
Copy link

@SukkaW I Knew it.But when I use Hexo 4.2, the page can't render toc correctly.So I edit tocObj for now, and it works.

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 a pull request may close this issue.

4 participants