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

refactor(toc_obj): simplify the code #181

Merged
merged 1 commit into from Feb 17, 2020
Merged

Conversation

@SukkaW
Copy link
Member

SukkaW commented Feb 6, 2020

No description provided.

@SukkaW SukkaW requested a review from hexojs/core Feb 6, 2020
min_depth: 1,
max_depth: 6
}, options);

const headingsSelector = ['h1', 'h2', 'h3', 'h4', 'h5', 'h6'].slice(options.min_depth - 1, options.max_depth).join(',');
const headingsSelector = ['h1', 'h2', 'h3', 'h4', 'h5', 'h6'].slice(min_depth - 1, max_depth);

This comment has been minimized.

Copy link
@SukkaW

SukkaW Feb 6, 2020

Author Member

Array has includes() as well, remove unnecessary .join().

This comment has been minimized.

Copy link
@curbengh

curbengh Feb 17, 2020

Contributor

Array also works better since the value must be exact match.
Currently headingsSelector also matches h tag.

if (!headings.length) return result;

for (const el of headings) {
for (let i = 0; i < headingsLen; i++) {

This comment has been minimized.

Copy link
@SukkaW

SukkaW Feb 6, 2020

Author Member

Use classic loop.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Feb 6, 2020

Coverage Status

Coverage decreased (-0.005%) to 96.994% when pulling 5a92d46 on SukkaW:optimize-toc-obj into 7e084de on hexojs:master.

@SukkaW SukkaW merged commit 8615f15 into hexojs:master Feb 17, 2020
2 of 3 checks passed
2 of 3 checks passed
coverage/coveralls Coverage decreased (-0.005%) to 96.994%
Details
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.