Skip to content

refactor highlight: add extend api for highlight#5095

Merged
stevenjoezhang merged 14 commits intomasterfrom
highlight
Dec 14, 2022
Merged

refactor highlight: add extend api for highlight#5095
stevenjoezhang merged 14 commits intomasterfrom
highlight

Conversation

@stevenjoezhang
Copy link
Member

@stevenjoezhang stevenjoezhang commented Nov 5, 2022 •
edited
Loading

What does it do?

Currently, there are 3 files relevant to code highlighting

lib/plugins/filter/before_post_render/backtick_code_block.js
lib/plugins/tag/code.js
lib/plugins/tag/include_code.js

Each file is hardcoded with support for highlight.js and prismjs, which would make it very difficult to support other code highlighting functions.

To make it more flexible, I propose a new highlight filter:

hexo.extend.highlight.register('highlight.js', highlighter);

declare function highlighter(code: string, options: {
  language?: string,
  meta?: string,
  highlightLines?: unknown, // TBD
  lineNumbers?: boolean,
  // ...
}): string

With this filter, developers can use custom code highlighting functions to replace the built-in highlight.js or prismjs.

Issue resolved: #1891 #1300 #4010 #1938

Usage

// _config.yml
highlighter: 'prismjs'

Screenshots

Pull request tasks

  • Add test cases for the changes.
  • Passed the CI test.

@github-actions
Copy link

github-actions bot commented Nov 5, 2022 •
edited
Loading

Publish flamegraph to https://ffdeaee6503ad5877006576f59622fb7d91390cc-16-hexo.surge.sh/flamegraph.html

@coveralls
Copy link

coveralls commented Nov 5, 2022 •
edited
Loading

Coverage Status

Coverage increased (+0.09%) to 98.769% when pulling 46021e0 on highlight into 40df922 on master.

renbaoshuo
renbaoshuo previously approved these changes Nov 5, 2022
View reviewed changes
Copy link
Member

@renbaoshuo renbaoshuo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM

@stevenjoezhang
Copy link
Member Author

stevenjoezhang commented Nov 5, 2022

Discussion: should we use filter api to support highlight event, or create a new hexo.extend.highlight api? See #4010 (comment)

CC @hexojs/core

@renbaoshuo
Copy link
Member

renbaoshuo commented Nov 5, 2022 •
edited
Loading

I think we should use hexo.extend.filter.register('highlight', ...). Because code highlighting is a (sometimes optional) step in the article rendering process.

@stevenjoezhang
Copy link
Member Author

stevenjoezhang commented Nov 5, 2022

Another difference is: other filters allow multiple callback functions to be registered per event, and callbacks are executed sequentially when the event is triggered. For code highlighting, only a single handler function should be registered. (For example, the user can choose either highlight.js or prism, but should not register both with hexo.extend.filter)

SukkaW
SukkaW requested changes Nov 5, 2022
View reviewed changes
Copy link
Member

@SukkaW SukkaW left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO, the ideal design is that we have a new plugin API hexo.extend.highlight:

hexo.extend.highlight.register('highlight.js', highlighter);
hexo.extend.highlight.register('prismjs', highlighter);
hexo.extend.highlight.register('shiki', highlighter);

We can read _config.yml to determine which highlighter to use:

// _config.yml
highlight: 'shiki'

The highlighter should have an API interface like this:

declare function highlighter({
  code: string,
  language?: string,
  meta?: string,
  highlightLines?: unknown, // TBD
  lineNumbers?: boolean,
  // ...
}): string

@stevenjoezhang stevenjoezhang requested a review from SukkaW November 5, 2022 09:17
@stevenjoezhang
Copy link
Member Author

stevenjoezhang commented Nov 5, 2022 •
edited
Loading

New API interface of highlighter:

declare function highlighter(code: string, options: {
  language?: string,
  meta?: string,
  highlightLines?: unknown, // TBD
  lineNumbers?: boolean,
  // ...
}): string

The types of two parameters are the same as require('hexo-util').highlight and require('hexo-util').prismHighlight

Besides, can I remove the following code? It seems to be dead code

hexo/lib/hexo/post.js

Line 409 in 871ac4f

if (!config.highlight.enable) options.highlight = null;

See also hexojs/hexo-renderer-marked#134 https://github.com/hexojs/hexo-renderer-marked/issue/26

@stevenjoezhang stevenjoezhang changed the title refactor highlight: add filter event for code block rendering refactor highlight: add extend api for highlight Nov 5, 2022
@stevenjoezhang stevenjoezhang requested a review from a team November 7, 2022 13:54
@yoshinorin yoshinorin added this to the 7.0.0 milestone Nov 21, 2022
This was referenced Nov 22, 2022
Closed
Merged
yoshinorin
yoshinorin reviewed Dec 13, 2022
View reviewed changes
yoshinorin
yoshinorin previously approved these changes Dec 13, 2022
View reviewed changes
Copy link
Member

@yoshinorin yoshinorin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggested two comments, but basically LGTM. I think we can merge this.

@yoshinorin
Copy link
Member

yoshinorin commented Dec 13, 2022 •
edited
Loading

@stevenjoezhang

Would you please change the filename from lib/extend/highlight.js to lib/extend/syntaxhighlight.js ?

30403c1

yoshinorin
yoshinorin approved these changes Dec 14, 2022
View reviewed changes
Copy link
Member

@yoshinorin yoshinorin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@stevenjoezhang stevenjoezhang merged commit d485ebd into master Dec 14, 2022
@stevenjoezhang stevenjoezhang deleted the highlight branch December 14, 2022 14:36
@renbaoshuo renbaoshuo mentioned this pull request Jan 8, 2023
Closed
2 tasks
@stevenjoezhang stevenjoezhang mentioned this pull request Feb 11, 2023
Closed
5 tasks
@prinsss prinsss mentioned this pull request Nov 7, 2023
Closed
5 tasks
@stevenjoezhang stevenjoezhang mentioned this pull request Apr 18, 2025
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yoshinorin yoshinorin yoshinorin approved these changes

@renbaoshuo renbaoshuo renbaoshuo left review comments

@SukkaW SukkaW Awaiting requested review from SukkaW

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

7.0.0

Development

Successfully merging this pull request may close these issues.

Add filter event for backtick code block

5 participants

@stevenjoezhang @coveralls @renbaoshuo @yoshinorin @SukkaW