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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
