Colonial-Dev / inkjet

A batteries-included syntax highlighting library for Rust, based on tree-sitter.
https://docs.rs/inkjet
Apache License 2.0
60 stars 4 forks source link

[Proposal] Nvim Themes #27

Closed leandrocp closed 1 day ago

leandrocp commented 2 days ago

Hey @Colonial-Dev version 0.11 is exciting!

But reading https://github.com/Colonial-Dev/inkjet/issues/25 I realized you might face the same issues I'm facing on https://github.com/leandrocp/autumn regarding themes.

In short, the issue I see the most is a mismatch between theme highlights and the highlights generated by queries, and part of that is due to using more upstream queries instead of the ones provided by Helix, since Helix themes are built mostly for the queries available in Helix itself.

Take for instance the Elixir language, in Helix the alias is highlighted as @namespace while in the official package, it's defined as @module:

https://github.com/helix-editor/helix/blob/5ce77de0dc7106c6f1460d80a3c5a51eaea3108c/runtime/queries/elixir/highlights.scm#L85

https://github.com/elixir-lang/tree-sitter-elixir/blob/2c6e93171477973b38de4b1d2be427cc99f990a6/queries/highlights.scm#L193

That's just one example but you can find many other mismatches in other languages.

The initial thought would be to revert to Helix queries only but we know many outdated or incomplete, which leads to "I've moved ~half of the languages to their upstream queries as things improved or bugs cropped up" like you said, which makes sense because even GitHub (afaik) is using the official upstream queries on github.com itself.

So I was wondering how to keep queries updated and provide themes that match the generated highlights, and a possible solution is using Neovim themes instead of helix themes. For a few reasons:

Themes in Neovim are defined in Lua files so first we'd need to extract the highlights and styles, which can be done with a script that executes a headless nvim instance without plugins, installs the themes, apply the colorscheme and call the nvim api to extract the style of each highlight. I already wrote a script based on https://github.com/RubixDev/syntastica/tree/main/syntastica-themes to prove this idea, and it does generates a .toml for each theme similar to the one we have in Helix.

So in short:

I'm aware that's a deeper change in Inkjet core so I totally understand if you don't want to go down that route :) But if you think that makes sense I can push a PR with the initial work. Thanks!

guilhermeprokisch commented 2 days ago

@leandrocp can you drop a couple of these .toml files for test?

Thanks

leandrocp commented 2 days ago

@guilhermeprokisch there you go https://gist.github.com/leandrocp/1a6d891b992be24ceb8c6346126b32a4

In that gist you'll also find the script to extract the colorscheme, which you can call with nvim --headless -u NONE -l extract_themes.lua folke/tokyonight.nvim tokyonight-day

Keep in mind that's still a draft and might have a bug or two, although it seems to be working well with some of the most popular themes.

leandrocp commented 2 days ago

Btw @guilhermeprokisch would be interesting to validate that idea on https://github.com/guilhermeprokisch/see if you have the time.

guilhermeprokisch commented 2 days ago

My two cents.

Colonial-Dev commented 2 days ago

My concern is that (unless I'm missing something) the Neovim highlights aren't standardized either. There isn't really a "standard" at all; even official tree-sitter implementations differ on what they recognize, and generally aren't as comprehensive as the third party implementations (see the tree_sitter_highlight crate, for example. carriage_return?!)

Definitely worth a try, though. It might end up "just working" due to Neovim's popularity, as you point out.

If it doesn't, there's also:

Colonial-Dev commented 2 days ago

I would like to keep the Helix theme support around, even if we don't bundle the themes anymore. The [palette] functionality is very nice if you want to make a theme from scratch or even modify an existing one.

(Fortunately this can probably be handled transparently with a bit of Serde magic - no need for a separate method, even.)

leandrocp commented 1 day ago

Actually giving a second thought, only one small change is needed to support nvim themes. We just need to include the extra highlights listed here https://github.com/nvim-treesitter/nvim-treesitter/blob/master/CONTRIBUTING.md#highlights - it won't break existing Helix themes nor will add too much overhead (it's just a few more highlights).

Then in my library, I can convert nvim themes to .toml files in the format expected by Inkjet and call Theme::from_helix as usual.

If that works and you like the result, eventually I could push those themes upstream. But for now we could start with the smallest change possible in Inkjet.

Does that work for you?


And to answer some of your concerns (and keep it here for future reference):

Neovim highlights aren't standardized either

It's as much standardized as Helix, they provide a guide that maintainers should follow, same as the Helix guide.

generally aren't as comprehensive as the third party implementations

Yep, I think most editors/tools implement their own HIGHLIGHT_NAMES and that one in tree-sitter-highlight create is just a generic starting point.

It might end up "just working" due to Neovim's popularity, as you point out.

I think that's precisely the benefit :)

Since nvim-treesitter is using official parsers from upstream (edit: I meant the queries in nvim-treesitter are more aligned with upstream) and they have a larger community to maintain the themes, that seems to me the best combo we can have. Take for instance the Dracula theme that @guilhermeprokisch mentioned in another issue, even tho it's a popular theme the colors won't be as accurate as it's in nvim and vscode - https://github.com/dracula/dracula-theme/issues/917

If it doesn't, there's also

I don't think it would even get to the point of doing suck workarounds. Most (if not all) themes support tree-sitter so that's it, just need to recognize a few extra highlights and extract the colors.

The [palette] functionality is very nice

Oh I agree 100% - actually I tried to automate extracting the palette as well since most nvim themes have one but it's not straightforward so for this first iteration I'm more concerned with matching the colors correctly, and eventually I can work on extracting the palette list too.

leandrocp commented 1 day ago

Hey I'm gonna close this issue for now because it will demand more tests to prove if it really works, essentially it does require loading lang queries from nvim-treesitter besides extracting the themes, which also requires a bit more work. So there's nothing to be done in Inkjet yet. I'll keep a branch on my fork and post updates here for those interested, it may or may not work :)

Colonial-Dev commented 9 hours ago

Does that work for you?

Yes, that sounds good! Ultimately I would like this issue to go away permanently (especially now that the personal project I originally created this crate for has since died and been resurrected) and this sounds like the right way to go.

I'll keep a branch on my fork and post updates here for those interested, it may or may not work :)

Good luck and thank you!