-
Notifications
You must be signed in to change notification settings - Fork 353
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
improvement: Remove semantic tokens fallback #5985
Conversation
8606fe3
to
a438ca1
Compare
a438ca1
to
e78e98e
Compare
@@ -171,7 +171,7 @@ class SemanticTokensLspSuite extends BaseLspSuite("SemanticTokens") { | |||
"self-type", | |||
"""|<<package>>/*keyword*/ <<a>>/*namespace*/ | |||
| | |||
|<<object>>/*keyword*/ <<Abc>>/*class*/ { <<self>>/*variable,readonly*/: <<Any>>/*class*/ <<=>>>/*operator*/ | |||
|<<object>>/*keyword*/ <<Abc>>/*class*/ { self: Any <<=>>>/*operator*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like Any
should be marked as class
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like no features work on Any
(actually its not specific to Any
, but I haven't yet figured the pattern) in self type, including hover and document highlight, because there is no symbol at this node in the AST.
I think its best to work on it in a follow up, since its not specifically related to semantic highlighting. I would still be coloured as class by the editor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to work with the self
type in line 192
, so I though it's a strictly Any
/AnyRef
type of situation. Anyway let's do it as a follow up .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in #6006
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Previously, for identifiers with no symbol found, we've added fallback semantic tokens based on simple heuristic.
This was causing some problems for not-compiling projects, probably best to remove it and leave syntax highlighting to an editor.