-
-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
feat(helper/toc): specify maximum number of items to output #5487
Conversation
How to testgit clone -b max-items-of-toc https://github.com/KentarouTakeda/hexojs-hexo.git
cd hexo
npm install
npm test |
Pull Request Test Coverage Report for Build 9729543737Details
💛 - Coveralls |
81952b5
to
0954b95
Compare
0954b95
to
f00bf65
Compare
It seems that the test is failing at some points unrelated to this pull request. Other pull requests have also failed, so I'll leave it as is. |
@@ -27,7 +29,7 @@ function tocHelper(str: string, options: Options = {}) { | |||
list_number: true | |||
}, options); | |||
|
|||
const data = tocObj(str, { min_depth: options.min_depth, max_depth: options.max_depth }); | |||
const data = getAndTruncateTocObj(str, { min_depth: options.min_depth, max_depth: options.max_depth }, options.max_items); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the user does not need max_items
, then there is no need to perform this operation everytime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed: c17db64
I thought it was necessary to avoid tocObj()
appearing multiple times in different places, so I wrote the skip judgment at the beginning of getAndTruncateTocObj()
instead of where you gave the example.
lib/plugins/helper/toc.ts
Outdated
const min = Math.min(...data.map(item => item.level)); | ||
const max = Math.max(...data.map(item => item.level)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...data.map(item => item.level)
They look the same,
I guess it doesn't need to be executed twice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed: f8d04a6
for (let currentLevel = max; data.length > max_items && currentLevel > min; currentLevel--) { | ||
data = data.filter(item => item.level < currentLevel); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be inefficient (iterating data over and over again). But I don't have any better idea for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation is inefficient. But since the benchmark doesn't show any regression, I am good with that.
Site needs to be updated |
@KentarouTakeda |
Of course! |
) * feat(helper/toc): specify maximum number of items to output * perf: skip evaluation if `max_items` is not specified * perf: streamline getting min and max heading level --------- Co-authored-by: Uiolee <22849383+uiolee@users.noreply.github.com> Co-authored-by: yoshinorin <yoshinorin.net@outlook.com>
What does it do?
Added the ability to set an upper limit on the number of items in the table of contents output by the
toc()
helper.If no value is specified, the default is
Infinity
. This is no different from previous behavior. If we specify a number greater than or equal to 1, the elements will be truncated in the following order.<h6>
to<h1>
) to fit within the specified number.For example, if we have a heading like this:
If
5
is specified formax_items
, all items will be output. On the other hand, if we specify4
or3
,All
<h3>
are removed, resulting in two items being output. This is the same result as specifying2
formax_items
.This is because this outline requires five elements to display up to
<h3>
.Only the highest heading that appears on the page behaves differently. For example, if we specify
1
formax_items
,Instead of all
<h2>
being deleted at once as before, they will be deleted sequentially from the bottom. This is to avoid unintentionally not outputting anything.In addition to the code, I welcome any feedback regarding the specifications described here. I believe this specification is the best, but if you have any other good ideas, I would definitely consider them.
Screenshots
Pull request tasks