[PATCH] D65637: [clangd] [WIP] Semantic highlighting prototype for the vscode extension.

[Johan Vikström via Phabricator via cfe-commits]

jvikstrom marked an inline comment as done.
jvikstrom added inline comments.


================
Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/TextMate.ts:20
+            const split = scope.split('.');
+            while(split.length > 0) {
+                split.splice(-1)
----------------
hokein wrote:
> jvikstrom wrote:
> > I'm pretty sure this matching logic is wrong. An example of the scopes for variables in the Light+ theme:
> > 
> > ```
> > variable.css
> > variable.scss
> > variable.other.less
> > variable.language.wildcard.java
> > variable.language
> > storage.type.variable.cs
> > variable
> > meta.definition.variable.name
> > support.variable
> > entity.name.variable
> > ```
> > 
> > With this kind of matching we have now this means that `variable.other.less` will match `variable.other` and `variable.other.less`.
> > As there is no perfect match for `variable.other.field` it will try to match `variable.other` and find the color for less.
> > 
> > What should probably be done is remove the divided map. The query function should still be the same but  query on this.colors instead of this.divided.
> The suffix of textmate indicates the language it belongs to (e.g. java), I think we can ignore non-C++ textmates, and merely find the longest-prefix match in all `c++` textmates?
But we have no real way of knowing if a suffix is a language or a more "precise" scope.
I don't think it's really possible to differentiate between `variable.other` and `variable.css` and say that `variable.css` is not relevant to c++ while `variable.other` is. (other than having an exhaustive list of language suffixes, which isn't feasible)

The implementation I changed to just saves the full scopes and than when we try to find the color of a scope we try to find it in the map, otherwise remove a suffix and try to find that scope (over and over)

Ex: Find scope for `variable.other.field.cpp` -> 
  First check for `variable.other.field.cpp` (finds nothing)
  Check `variable.other.field` (finds nothing)
  Check `variable.other` (still finds nothing
  Check `variable` (finds the variable scope and returns that color)

This seems to work, but maybe there's some case I miss. Anyway, there is probably a defined order on how you are supposed to find these scopes, I think I read a blog post on how VSCode actually does this matching internally. I'll try to dig that up and make sure our implementation matches before it's time to put up the actual CLs for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65637