-
-
Notifications
You must be signed in to change notification settings - Fork 5k
refactor(toc): support skipping heading level #5653
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
base: master
Are you sure you want to change the base?
Conversation
How to testgit clone -b toc https://github.com/hexojs/hexo.git
cd hexo
npm install
npm test |
Pull Request Test Coverage Report for Build 16493028975Details
💛 - Coveralls |
dfdecdb
to
ce6512d
Compare
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.
Pull Request Overview
This PR refactors the Table of Contents (TOC) helper to support skipping heading levels by replacing the previous linear processing approach with a tree-based structure. The refactor addresses issue #2137 by properly handling cases where heading levels are not sequential (e.g., h1 followed directly by h3).
Key changes:
- Replaced linear TOC generation with tree-based approach using
buildTree()
function - Separated numbering logic into dedicated
assignNumbers()
function - Added comprehensive test case for skipped heading levels scenario
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
File | Description |
---|---|
lib/plugins/helper/toc.ts | Complete refactor from linear to tree-based TOC generation with new helper functions |
test/scripts/helpers/toc.ts | Added test case verifying correct handling of skipped heading levels |
* @param {string} str Raw markdown/html string | ||
* @param {Options} options Configuration options | ||
*/ | ||
function tocHelper(str, options: Options = {}) { |
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 str
parameter should have explicit type annotation string
to maintain consistency with TypeScript best practices and the original function signature.
function tocHelper(str, options: Options = {}) { | |
function tocHelper(str: string, options: Options = {}) { |
Copilot uses AI. Check for mistakes.
/** | ||
* Extract flat TOC data and enforce max_items | ||
*/ | ||
function getAndTruncateTocObj(str, { min_depth, max_depth }, 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.
Function parameters lack type annotations. Should be str: string
, { min_depth, max_depth }: {min_depth: number, max_depth: number}
, and max_items: number
to maintain TypeScript type safety.
function getAndTruncateTocObj(str, { min_depth, max_depth }, max_items) { | |
function getAndTruncateTocObj(str: string, { min_depth, max_depth }: { min_depth: number, max_depth: number }, max_items: number) { |
Copilot uses AI. Check for mistakes.
/** | ||
* Build nested tree from flat heading list | ||
*/ | ||
function buildTree(headings) { |
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 headings
parameter should have explicit type annotation to maintain TypeScript consistency. Consider adding appropriate interface or type definition.
function buildTree(headings) { | |
function buildTree(headings: Heading[]) { |
Copilot uses AI. Check for mistakes.
function assignNumbers(nodes) { | ||
const counters = []; | ||
function dfs(list, depth) { |
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 nodes
parameter should have explicit type annotation to maintain TypeScript consistency. Consider adding appropriate interface or type definition.
function assignNumbers(nodes) { | |
const counters = []; | |
function dfs(list, depth) { | |
function assignNumbers(nodes: Node[]) { | |
const counters: number[] = []; | |
function dfs(list: Node[], depth: number) { |
Copilot uses AI. Check for mistakes.
test/scripts/helpers/toc.ts
Outdated
'<h1>Title 1</h1>' | ||
].join(''); | ||
|
||
toc(input).should.eql('<ol class="toc"><li class="toc-item toc-level-1"><a class="toc-link"><span class="toc-number">1.</span> <span class="toc-text">Title 1</span></a><ol class="toc-child"><li class="toc-item toc-level-3"><a class="toc-link"><span class="toc-number">1.1.</span> <span class="toc-text">Title 3</span></a><ol class="toc-child"><li class="toc-item toc-level-4"><a class="toc-link"><span class="toc-number">1.1.1.</span> <span class="toc-text">Title 4</span></a></li></ol></li><li class="toc-item toc-level-2"><a class="toc-link"><span class="toc-number">1.2.</span> <span class="toc-text">Title 2</span></a><ol class="toc-child"><li class="toc-item toc-level-5"><a class="toc-link"><span class="toc-number">1.2.1.</span> <span class="toc-text">Title 5</span></a></li></ol></li></ol></li><li class="toc-item toc-level-1"><a class="toc-link"><span class="toc-number">2.</span> <span class="toc-text">Title 1</span></a></li></ol>'); |
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 extremely long assertion string (over 500 characters) is difficult to read and maintain. Consider breaking it into a multi-line template literal or extracting it to a variable for better readability.
toc(input).should.eql('<ol class="toc"><li class="toc-item toc-level-1"><a class="toc-link"><span class="toc-number">1.</span> <span class="toc-text">Title 1</span></a><ol class="toc-child"><li class="toc-item toc-level-3"><a class="toc-link"><span class="toc-number">1.1.</span> <span class="toc-text">Title 3</span></a><ol class="toc-child"><li class="toc-item toc-level-4"><a class="toc-link"><span class="toc-number">1.1.1.</span> <span class="toc-text">Title 4</span></a></li></ol></li><li class="toc-item toc-level-2"><a class="toc-link"><span class="toc-number">1.2.</span> <span class="toc-text">Title 2</span></a><ol class="toc-child"><li class="toc-item toc-level-5"><a class="toc-link"><span class="toc-number">1.2.1.</span> <span class="toc-text">Title 5</span></a></li></ol></li></ol></li><li class="toc-item toc-level-1"><a class="toc-link"><span class="toc-number">2.</span> <span class="toc-text">Title 1</span></a></li></ol>'); | |
toc(input).should.eql(` | |
<ol class="toc"> | |
<li class="toc-item toc-level-1"> | |
<a class="toc-link"> | |
<span class="toc-number">1.</span> <span class="toc-text">Title 1</span> | |
</a> | |
<ol class="toc-child"> | |
<li class="toc-item toc-level-3"> | |
<a class="toc-link"> | |
<span class="toc-number">1.1.</span> <span class="toc-text">Title 3</span> | |
</a> | |
<ol class="toc-child"> | |
<li class="toc-item toc-level-4"> | |
<a class="toc-link"> | |
<span class="toc-number">1.1.1.</span> <span class="toc-text">Title 4</span> | |
</a> | |
</li> | |
</ol> | |
</li> | |
<li class="toc-item toc-level-2"> | |
<a class="toc-link"> | |
<span class="toc-number">1.2.</span> <span class="toc-text">Title 2</span> | |
</a> | |
<ol class="toc-child"> | |
<li class="toc-item toc-level-5"> | |
<a class="toc-link"> | |
<span class="toc-number">1.2.1.</span> <span class="toc-text">Title 5</span> | |
</a> | |
</li> | |
</ol> | |
</li> | |
</ol> | |
</li> | |
<li class="toc-item toc-level-1"> | |
<a class="toc-link"> | |
<span class="toc-number">2.</span> <span class="toc-text">Title 1</span> | |
</a> | |
</li> | |
</ol> | |
`); |
Copilot uses AI. Check for mistakes.
What does it do?
Issue resolved: #2137
Screenshots
Pull request tasks