-
-
Notifications
You must be signed in to change notification settings - Fork 460
feat: add plugin capability #381
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
Conversation
✅ Deploy Preview for shiki-matsu ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
I'm calling deciding on plugin support as being above my shiki pay-grade - I'm lightly in favour of this proposal though, I think it's a good way to let us as maintainers not be in the way of folks wanting to do their own interesting things. Plus, going plugin is literally in my TODO list for every big OSS project I end up maintaining: https://artsy.github.io/blog/2016/07/03/handling-big-projects/ |
Exactly my thinking. Been there too often to not start with extensibility in mind - and then you look for the knobs and handles to add/adjust for new functionality that's not part of the core. The existing functionality (swapping elements or line options) already help a lot. But then they move consumers away from the standard, and eventually future benefits. Btw - is it @octref's paygrade? ;-) |
This comment was marked as off-topic.
This comment was marked as off-topic.
|
||
You can also chain multiple plugins. The order of the plugins is important, as they are applied in the order they are passed to `usePlugin()`. | ||
|
||
> Note: After invoking `codeToHtml()` the plugins in the highlighter will be reset, so you will need to call `usePlugin()` again if you want to use the plugins again. |
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 feels strange to me. highlighter
is a singleton and reused over. Why would the plugins be reset after each highlighting?
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.
Good question.
When building this I was thinking about the use cases where someone will have multiple code blocks, and where they want different plugins applied to each block.
Like:
- Block 1 - line numbers
- Block 2 - line numbers and blur out some lines
- Block 3 - line numbers and diff highlighting
So it's mostly really about avoiding conflicting between blocks. The singleton would stay.
Happy to discuss better solutions. 🤞
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.
Maybe in a new version of the API, I can separate the async fetching and the synchronous construction of the highlighter, so you can easily recreate new instances of the highlighter and load each of them with different plugins.
Hey @muenzpraeger, thanks for your work and sorry for my late response. Has been busy with other things. Overall I think the PR is good. I'm planning to push for a 1.0 release and break some API, so this could be a good addition. I'll open a new issue for 1.0 plans soon. Re the PR itself, I want to strike a good balance between easy-to-use and extensibility. Here's a list of goals we should strive for:
If done well, yes. I think we got a solid start. Here's the next steps:
I do want to get this in, but I want to do things in sequence too. Be patient with me :p |
Exciting stuff, look forward to diff support |
@octref I'm all in! The current Plugin API design is based on that I didn't want to add a breaking change (b/c I'm not the project owner). I totally agree on that there should be a well-balanced set of built-in functionality. In that regard I'm a strong proponent of having that also built on a plugin architecture, so that it's dogfeeding into the core. LMK if you want a helping keyboard with v1. |
Another use case that I've seen is pre/post-processing of shiki. For example, rehype-pretty-code is a plugin for remark that uses shiki for processing. By making it part of the rehype toolchain, it makes it easy to process tokens and other systems (thanks wooorm!) In fact, this plugin system looks similar to the purpose of remark. Instead of engineering a new plugin system, it may be better to integrate remark plugins, providing compatibility with the vast available remark plugins. |
To clarify, I don't think it's a good idea to make an entirely new toolchain for HTML post/preprocessing. I think that encouraging the use of remark would be a good idea for additional HTML processing, while allowing some sort of token processing internally into shiki. |
Add a test if possible
Format all commit messages with Conventional Commits
This PR fixes Add plugin capability #380, fixes Dark mode support mentioned in docs does not work #373, fixes Support for diffs & code blur #330, fixes Custom <pre> class name? #206, fixes Line numbers #3
It also supersedes feat: add
addThemeNameToClass
option, fixes #373 #376.See the file
plugin.md
for usage details. I also provided diff plugin as example here. Note that the plugin is not published tonpm
yet, so for testing it needs to be linked locally.