Stream: t-compiler/wg-rls-2.0

Topic: rainbow mode


Laurențiu Nicola (Jul 19 2019 at 12:01, on Zulip):

Is it me, or is the rainbow mode a little unreliable?

pasted image

Laurențiu Nicola (Jul 19 2019 at 12:01, on Zulip):

also, it doesn't seem to handle toggling the preference

Laurențiu Nicola (Jul 19 2019 at 12:01, on Zulip):

pasted image

Laurențiu Nicola (Jul 19 2019 at 12:02, on Zulip):

here I disabled it so the underlines from #1550 show, but the colors don't change unless I re-open the editor window

matklad (Jul 19 2019 at 12:08, on Zulip):

Hm, yeah, it seems like rainbow mode is not properly cleared on modification...

Laurențiu Nicola (Jul 19 2019 at 12:09, on Zulip):

pasted image

Laurențiu Nicola (Jul 19 2019 at 12:09, on Zulip):

ugh, not sure if I broke it or it's wrong

matklad (Jul 19 2019 at 12:16, on Zulip):

So I see at least two problems

matklad (Jul 19 2019 at 12:17, on Zulip):
Laurențiu Nicola (Jul 19 2019 at 12:18, on Zulip):

yeah, that seems likely

Laurențiu Nicola (Jul 19 2019 at 12:18, on Zulip):

I can send you a patch for underlining in rainbow mode

matklad (Jul 19 2019 at 12:19, on Zulip):

cc @Pascal I am looking at this code, and this seems wrong to me (IIRC, you are the author):

https://github.com/rust-analyzer/rust-analyzer/blob/f209843e31af7f0e0212aa28ffec2efad2a70c6f/editors/code/src/highlighting.ts#L134-L137

We create a fresh decoration on every re-highlighting, but we never clear highlighting ranges

matklad (Jul 19 2019 at 12:20, on Zulip):

I think we should do one of the following:

Laurențiu Nicola (Jul 19 2019 at 12:20, on Zulip):

(:confused: why doesn't JS's Map have a way to avoid double lookups as inhas, then set?)

Pascal (Jul 19 2019 at 12:21, on Zulip):

@matklad oh, the highlights don't overwrite each other? :O i think that was my assumption when writing that

Laurențiu Nicola (Jul 19 2019 at 12:22, on Zulip):

apparently they're removed when the config changes, but only if the highlighting is disabled

Laurențiu Nicola (Jul 19 2019 at 12:22, on Zulip):

which is why toggling rainbow mode doesn't clear them

Pascal (Jul 19 2019 at 12:23, on Zulip):

hm. how do you clear decorations? if we can put these decs in a map and go through them before applying this might be super slow

Pascal (Jul 19 2019 at 12:24, on Zulip):

if we make the fancify function use less entropy we can probably get down to a few dozen colors, but i'm not an expert on this at all

matklad (Jul 19 2019 at 12:25, on Zulip):

hm. how do you clear decorations?

editor.setDecorations(dec, []). Perhaps there's an explicit method to dispose the decoration as well

Laurențiu Nicola (Jul 19 2019 at 12:27, on Zulip):

I can send you a patch for underlining in rainbow mode

ah, you've already implemented that (small nit: you can move textDecoration outside of the light/dark options)

Pascal (Jul 19 2019 at 12:28, on Zulip):

ok, how about this: adjust fancify so it uses a smaller range for h, s, and l (and expand with *3 at the end or something) and make the l component be flipped instead of two uniq random values. then build a Map<HSL, Decoration> and hope it's only ~300 entries?

Pascal (Jul 19 2019 at 12:30, on Zulip):

the advantage of using the random seeding is that it's stable across files so identifiers have the same color in two panels

Laurențiu Nicola (Jul 19 2019 at 12:30, on Zulip):

editor.setDecorations(dec, []). Perhaps there's an explicit method to dispose the decoration as well

there is

matklad (Jul 19 2019 at 12:32, on Zulip):

@Pascal sgtm!

Pascal (Jul 19 2019 at 12:34, on Zulip):

i won't have time to work on this for the next week or so but just ping me if you want me to look at stuff :)

matklad (Jul 19 2019 at 12:37, on Zulip):

sure, thanks!

Laurențiu Nicola (Jul 19 2019 at 12:40, on Zulip):

@matklad how do you feel about enabling trailing commas in the prettier config?

matklad (Jul 19 2019 at 12:41, on Zulip):

I am mildly in favor, but, mainly, I have no idea about typescript current best-pracices, so I am happy to just defer to anybody more knoledgable :)

Last update: Nov 12 2019 at 16:10UTC