[PATCH] D87669: [clangd] Rainbow Semantic Highlighting

Fri Sep 18 08:50:48 PDT 2020

alexzielenski added a comment.

Hey Sam, Thanks for checking this out!

I really like your idea about encoding the variants with binary - as you say it removes the need for any assumption on the server's part for the limit of colors. However, I'd be wary of the fact that most users and theme creators don't know or understand binary and probably also don't want to learn binary. Indeed - I dont think users would easily accept (or obey) being limited to a power of 2 for their rainbow variants - and be subject to ambiguous behavior that arises when variable.1.2 is requested to be highlighted by a theme with only variable.1 and variable.2 entries.

Additionally, having creating a theme myself, I've come to understand that anyone would be hard pressed to find 256 visually distinct colors that are still similar enough to recognize as the same semantic type. I had a hard enough time finding 5 good ones even with the help of automated tools.

So I think for the server to have a small imposed limit would be okay - the only practical drawback I can think of with my current solution is you are forced to have 5 colors: If `variable.4` is requested to be highlighted and you only have `variable.1`;`variable.2`;`variable.3` defined by a theme, my current system with just highlight this with the unmodified `variable`. The binary-encoded system you propose would not have this issue and would properly "modulo" for the defined bits. Of course this could be a configuration option, but I don't like that either.

My intent for this patch is to get the ball rolling - I'm not really interesting in bringing it through the review process since I get enough of that at my day job 😅 (I also adjusted the visibility to users only to try to stay discrete) but I am happy to help influence discussion for this patch. I've been following clangd for a few years and have been happy that since around clangd-10/11 it seems to have gotten usable for my day-to-day use. Tools like these 10x developer productivity so good work!

================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:606
+      Out.tokenModifiers |= 1 << 7;
+    }

----------------
This method of managing the modifiers is pretty obviously bad.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87669/new/

https://reviews.llvm.org/D87669