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 highlight: add extend api for highlight #5095

Merged
merged 14 commits into from
Dec 14, 2022
Merged

Conversation

stevenjoezhang
Copy link
Member

@stevenjoezhang stevenjoezhang commented Nov 5, 2022

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

@coveralls
Copy link

coveralls commented Nov 5, 2022

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
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

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

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

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)

Copy link
Member

@SukkaW SukkaW left a comment

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
Copy link
Member Author

stevenjoezhang commented Nov 5, 2022

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

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
lib/extend/highlight.js Outdated Show resolved Hide resolved
lib/hexo/default_config.js Outdated Show resolved Hide resolved
yoshinorin
yoshinorin previously approved these changes Dec 13, 2022
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

@stevenjoezhang

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

30403c1

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 🚀

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 this pull request may close these issues.

Add filter event for backtick code block
5 participants