helix icon indicating copy to clipboard operation
helix copied to clipboard

Rainbow tree-sitter matches 🌈

Open the-mikedavis opened this issue 2 years ago • 27 comments

Configurable, themable, nesting rainbow highlights:

demo

This approach uses the syntax tree to guide nesting levels and highlights so it ends up being much more flexible than a naive pair-matching implementation. For example, we can highlight </> when used in Rust type parameters like struct MyStruct<'a> { .. } and not mistakenly highlight </> comparison operators. We can also capture multiple nodes, for example the #, [ and ] characters in a Rust attribute like #[cfg(windows)] and none of these characters are required to be adjacent.

The cherry on top is that we can highlight according to the syntax tree's nesting information rather than whether characters surround each other. For example, in this TOML table, characters is a sub-table of editor.whitespace and the rainbow highlights reflect that fact.

theme = "default"

[editor.whitespace]
# <- red          ^ red
render = "all"
characters = { tab = "▸", newline = "◂" }
#            ^ yellow                   ^ yellow (comes after red in base16)

It also works across injected languages like javascript injected into HTML script tags.

(You can see each of these things in action in the gif above.)

How does it work?...

This implementation leverages tree-sitter queries. We use a QueryCaptures Iterator very similar to the existing one for syntax highlights to query the parsed syntax tree for new rainbows.scm patterns.

Two new captures - @rainbow.scope and @rainbow.bracket - track nesting level and capture syntax nodes for highlighting with rainbows. Specifically: @rainbow.scope pushes a RainbowScope on a stack. When @rainbow.bracket matches, we test that the captured node is a direct descendant of the current scope. If it is, we emit highlights for it according to the scope's level of nesting.


Configuration...

In the theme we add a rainbow key that takes a list of Styles. You can have however many you want and you can re-use colors from the palette or even throw modifiers in there:

# ~/.config/helix/themes/my_theme.toml

rainbow = ["#fc6b59", { fg = "lavender", bg = "comet" }, "lilac", { modifiers = ["reversed"] }]

[palette]
lavender = "#a4a0e8"
comet = "#5a5977"
lilac = "#dbbfef"

A default rainbow using the red, yellow, green, blue, cyan, and magenta terminal colors is used as a fallback.

Enable the rendering of rainbow brackets in your config.toml

# ~/.config/helix/config.toml
[editor]
rainbow-brackets = true

Or enable rendering per-language in languages.toml (this overrides the config.toml setting for any languages that set this value):

# ~/.config/helix/languages.toml
[[language]]
name = "scheme"
rainbow-brackets = true

[[language]]
name = "commonlisp"
rainbow-brackets = true

When the rainbow-brackets option is disabled, rainbow brackets are not rendered or computed.


Status...

The current implementation is working well. Take it for a spin and let me know if you find any bugs!

the-mikedavis avatar Jun 22 '22 04:06 the-mikedavis

Surprisingly little code to get this working 👍🏻

archseer avatar Jun 22 '22 09:06 archseer

Would it make sense to use the color palette colors as default values if the rainbow feature is enabled? That was my idea when i tried to work on that.

SoraTenshi avatar Jun 22 '22 12:06 SoraTenshi

That could work but usually the palette has a bunch of different ui colors too, so some deeply nested brackets could end up using backgrounds / grays which may look bad. I think a default rainbow palette not based on the current theme could work ok

the-mikedavis avatar Jun 22 '22 13:06 the-mikedavis

You could cheat and ignore any scopes that start_with("ui.")

archseer avatar Jun 23 '22 13:06 archseer

I suspect theme colors may not be good for a theme's rainbow because they might blend in and end up looking confusing. I'll give it a try though - that would take all the guesswork out of having to come up with a default that looks good on all themes 😄

the-mikedavis avatar Jun 23 '22 13:06 the-mikedavis

You could get fancy and calculate the color contrast and only pick colors that are distant enough :D

archseer avatar Jun 24 '22 11:06 archseer

You could get fancy and calculate the color contrast and only pick colors that are distant enough :D

well, I have a snippet to calculate the L* luminance value (as in Lab/LCH "professional" colour systems) from an sRGB triple. I use it for exactly that, grading and excluding colours with weak contrast in a CLI app :+1:

if interested Helix would be most welcome to it, I can PR it to @the-mikedavis rainbow branch?

atagen avatar Jun 25 '22 06:06 atagen

Let's keep this branched focused just on the happy path for now (rainbows enabled and themed). If anything, we might want to merge this once cleaned up without any default behavior - i.e. the theme needs to have a rainbow key in order for rainbows to work - and we can find a good default palette or strategy as a follow-up.

the-mikedavis avatar Jun 25 '22 15:06 the-mikedavis

The reason I lean towards a hardcoded default that uses red/orange?/yellow/green/blue/purple/etc is that it's easy to get a feel for how things are nested when you can predict the ordering of the bracket colors. If I know blue comes after green, I know where a blue tuple stands inside a huge green map or list literal.

I don't want to discourage discussion on automatically determining a good rainbow though, it sounds like there are some cool color math tricks we could use 🙂

the-mikedavis avatar Jun 25 '22 16:06 the-mikedavis

~There's some not ideal behavior where the rainbow calculation depends on how much of the syntax tree is within view: https://asciinema.org/a/505048~

~I think it might work to scan back in the document until you reach some byte that belongs only to the root node of the syntax tree and start the highlight iterator from there (discarding highlights that are not in view). That would also help with https://github.com/helix-editor/helix/issues/1151 but I don't know how feasible or efficient it would be.~

~Edit: fixed in a78735b but I'll cherry-pick that out to its own PR for discussion. https://asciinema.org/a/507564~ #3026

Edit: fixed by splitting the syntax highlight iterator and rainbow highlight iterators, starting the rainbow iterator back far enough in the tree to reset the nesting level, and then merging the two highlight iterators together.

the-mikedavis avatar Jun 29 '22 23:06 the-mikedavis

I found an alternate approach for this that ends up being less complicated. Now it works similarly to the locals tracking system in the syntax highlights iterator. The queries end up being more intuitive to write and more flexible (see the updated PR description).

I think this approach is general-purpose enough: I've worked through 27 languages so far with the toughest being HTML. HTML works now so that nested XML elements behave like other nesting scopes rather than just highlighting the < and > within individual tags.

the-mikedavis avatar Aug 15 '22 04:08 the-mikedavis

I merged this with #4042 in 08e990c. The merge was nontrivial becasue I had to refactor the RainbowIter to emit Spans instead of HighlightEvent (and the history is not compatible with this branch I think). Should I open a new PR or how do you want to handles this @the-mikedavis?

pascalkuthe avatar Oct 01 '22 11:10 pascalkuthe

I'll rebase this once #4042 is merged. It's too soon to rebase now I think.

the-mikedavis avatar Oct 01 '22 13:10 the-mikedavis

I am also curious where do you get those example.*.

pickfire avatar Oct 05 '22 17:10 pickfire

I am also curious where do you get those example.*.

Most of them are from this repo:

  • example.toml: /languages.toml
  • example.rs: /helix-core/src/syntax.rs (in this branch)
  • example.scm: /runtime/queries/elixir/highlights.scm
  • example.html: /book/theme/index.hbs generated into an HTML file

And example.erl is https://github.com/erlang/otp/blob/1f10f9e8b497f7efc6da74f58e12b02b8ef1a431/lib/compiler/src/beam_disasm.erl#L218-L265 if I remember correctly.

Then I made a few screenshots of the same Wayland geometry (set a geometry variable with slurp and then grim -g $geometry frame-n.png for each example), and stitched them into a gif with ImageMagick: convert -delay 300 -loop 0 frame-*.png out.gif

the-mikedavis avatar Oct 05 '22 17:10 the-mikedavis

btw - would you require some assistance with the languages? I think i would be able to do it for Zig. As i am rather familiar with the language.

SoraTenshi avatar Oct 13 '22 15:10 SoraTenshi

That would be great! This PR has a guide for creating the rainbows.scm queries: https://github.com/helix-editor/helix/pull/2857/files#diff-68d1eecf9ab4c03f71757e5839fade60b6e110bf57713e6bebb2edef6a6dd062 and I could use some feedback on that, plus better language support would be good 👍

the-mikedavis avatar Oct 13 '22 15:10 the-mikedavis

It works well on this exact commit but if I rebase on master highlights are totally gone for rust

I can give this a rebase. IIRC the rust grammar updated and changed a node that was added in the rust rainbow queries which is probably causing the highlights to fail analysis.

I worry a bit that this PR introduces too much code duplication between HighlighIterator and RainbowIterator. I think a lot of what is solved by duplicating the highlight iterator here could be achieved using a trait.

Yeah I agree, there's quite a lot of duplication. I'll look into combining them and using a trait 👍

the-mikedavis avatar Dec 12 '22 18:12 the-mikedavis

Ok I was able to eliminate most of the duplicated code and shortened/simplified the approach. The new version adds an iterator over query captures across injections. Figuring out rainbow highlights then becomes a small wrapper over that. We could use the new iterator for textobject queries too and fix https://github.com/helix-editor/helix/issues/4618. I'll leave that as a follow-up PR though since it's unrelated.

the-mikedavis avatar Dec 16 '22 19:12 the-mikedavis

sorry about the delay with finishing my review, I am a bit busy in the days leading up to christmas and didn't get the chance to sit down for a longer period and do a thorough review. I will try to finish the review either in the evening today or tomorrow morning

pascalkuthe avatar Dec 20 '22 12:12 pascalkuthe

Just tested it out and it works very well, what's the hold-up?

subterfugue avatar Jan 28 '23 19:01 subterfugue

We're hoping to get #5420 in first since it's a large refactor and touches a lot of the rendering code. This will need to be rebased after that and I need to check if the queries need any fixing after recent tree-sitter update PRs.

In general though, a PR working well is a minimum requirement. We also need to check the code-quality and performance and decide whether the feature is something we want to support in core (rather than a plugin). All of that takes time, especially for a large and kinda complicated PR like this. There are a lot of PRs open that need a lot of review time and all of the maintainers review in their free time, so please be patient.

If you want to use unmerged stuff you can merge branches like this into a branch in a fork and build from that. This branch is in my "daily driver" branch as an example. I'll rebase and sort out the conflicts now to make that easier.

the-mikedavis avatar Jan 28 '23 20:01 the-mikedavis

If you want to use unmerged stuff you can merge branches like this into a branch in a fork and build from that. This branch is in my "daily driver" branch as an example. I'll rebase and sort out the conflicts now to make that easier.

Yup, that's e.g. what i did. i have an experimental branch up which is also used by quite some people. The code there is just an ugly merge of some prs of features i really want to use, but functional.

SoraTenshi avatar Jan 28 '23 20:01 SoraTenshi

You are probably already aware of this, but some of the languages like racket and common-lisp are missing a rainbows.scm file. They can just likely inherit from scheme like they do for highlights etc. Mentioning this because these are some of the languages that would benefit most from this feature.

paul-scott avatar Apr 05 '23 04:04 paul-scott

@paul-scott I think @the-mikedavis don't really have to go to do each one of them, especially those he don't even use, but other people can always add it for the other languages, just like inlay-hint, the author of inlay-hint, probably don't want to go add support for inlay-hint in each theme.

pickfire avatar Apr 07 '23 00:04 pickfire

Yea that makes sense

On Fri, 7 Apr 2023, at 10:55, Ivan Tham wrote:

@paul-scott https://github.com/paul-scott I think @the-mikedavis https://github.com/the-mikedavis don't really have to go to do each one of them, especially those he don't even use, but other people can always add it for the other languages, just like inlay-hint, the author of inlay-hint, probably don't want to go add support for inlay-hint in each theme.

— Reply to this email directly, view it on GitHub https://github.com/helix-editor/helix/pull/2857#issuecomment-1499800756, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPBNSNTJTDD7V4P6JWIZ5LW75QYLANCNFSM5ZOZY7AQ. You are receiving this because you were mentioned.Message ID: @.***>

paul-scott avatar Apr 07 '23 01:04 paul-scott

Good call with racket & common-lisp, I added those to the PR. I will probably avoid adding more languages myself unless I use them since it adds to the already large PR size and I'd like to see if contributors can use the new guide to write some queries themselves. But racket and common-lisp can just inherit as you say so it's no issue to add them 👍

the-mikedavis avatar Apr 13 '23 21:04 the-mikedavis

What's the status on this?

nrabulinski avatar Aug 16 '23 07:08 nrabulinski

There are a lot of conflicts with latest master. It seems like there will be more once #8021 is merged, as one of the conflicts I tried to resolve locally had to do with redraw. Perhaps the merge of that would be a good time to revisit and rebase this properly?

I'm interested in seeing #5176 progress, and it seems like the intent there is to build on the iterator improvements that are done here. It does look like this one is intended to merge before next release if the next milestone is to be believed.

Once the above cascade of merges is complete, we would have a pretty robust core tree-sitter implementation in editor, which will be a prerequisite for a lot of other cool functionality like #1252.

EpocSquadron avatar Aug 27 '23 16:08 EpocSquadron

The next milestone is only a set of PRs we intend to likely merge at some point/that we don't want to loose track of. But not indicative that PRs will necessarily make it into the next release (or block that). I don't think this will conflict much with #8021, the two PRs modify completely orthogonal parts of the codebase.

pascalkuthe avatar Aug 27 '23 16:08 pascalkuthe