Skip to content

Conversation

muenzpraeger
Copy link
Collaborator

@muenzpraeger muenzpraeger commented Dec 6, 2022

See the file plugin.md for usage details. I also provided diff plugin as example here. Note that the plugin is not published to npm yet, so for testing it needs to be linked locally.

@netlify
Copy link

netlify bot commented Dec 6, 2022

Deploy Preview for shiki-matsu ready!

Name Link
🔨 Latest commit 98b9be6
🔍 Latest deploy log https://app.netlify.com/sites/shiki-matsu/deploys/63a2da1969d31f0008262a78
😎 Deploy Preview https://deploy-preview-381--shiki-matsu.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@orta
Copy link
Contributor

orta commented Dec 18, 2022

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/

@muenzpraeger
Copy link
Collaborator Author

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.

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? ;-)

@bogdansoare

This comment was marked as off-topic.

@muenzpraeger
Copy link
Collaborator Author

@octref @orta Thoughts about this PR?


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.
Copy link
Collaborator

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?

Copy link
Collaborator Author

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

Copy link
Collaborator

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.

@octref
Copy link
Collaborator

octref commented Jan 26, 2023

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:

  • There should be one obvious way to do everything
  • The options API should suffice for as much use cases as possible (90%+)
  • Core functionalities should be part of the main package (be it done as options or bundled plugins). My thinking on this has changed and I feel diff and dark mode support should be built in.
  • The plugin API, with its dynamic hooks, should encapsulate more complex use cases. For example I'd love the effect of https://fatihkalifa.com/typescript-twoslash to be a userland plugin, but diff should be a core plugin. twoslash could be a core plugin that's not included/bundled by default, but can be easily added on.

It builds an ecosystem.

If done well, yes. I think we got a solid start. Here's the next steps:

  • Fix other smaller bugs and merge PRs
  • Feature freeze and release a last 0.x version
  • Flesh out a better version of the current options API
  • Review this proposal in light of the new API

I do want to get this in, but I want to do things in sequence too. Be patient with me :p

@octref octref mentioned this pull request Jan 26, 2023
10 tasks
@charnary
Copy link

Exciting stuff, look forward to diff support

@muenzpraeger
Copy link
Collaborator Author

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

@tristan-f-r
Copy link
Contributor

tristan-f-r commented Mar 28, 2023

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.

@tristan-f-r
Copy link
Contributor

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.

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 plugin capability Dark mode support mentioned in docs does not work Support for diffs & code blur Custom <pre> class name? Line numbers
6 participants