-
Notifications
You must be signed in to change notification settings - Fork 180
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
Support custom semantic tokens & modifiers #958
Conversation
869e457
to
87fc3f4
Compare
95ca56c
to
63e81e5
Compare
63e81e5
to
aff8a1f
Compare
aff8a1f
to
939ac49
Compare
939ac49
to
0cd8151
Compare
"scopes": { | ||
"hcl-attrName": ["variable.other.property"], | ||
"hcl-blockType": ["entity.name.type"], | ||
"hcl-blockLabel": ["variable.other.enummember"], | ||
|
||
"hcl-bool": ["keyword.control"], | ||
"hcl-string": ["string"], | ||
"hcl-number": ["constant.numeric"], | ||
"hcl-objectKey": ["variable.parameter"], | ||
"hcl-mapKey": ["variable.parameter"], | ||
"hcl-keyword": ["keyword.control"], | ||
"hcl-traversalStep": ["variable.other.readwrite"], | ||
"hcl-typeCapsule": ["keyword.control"], | ||
"hcl-typePrimitive": ["keyword.control"] | ||
} |
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.
The scopes here just follow VSCode's defaults per https://github.com/microsoft/vscode/blob/02e8bd149ca3fb68e7f26ebec33f920ba0feef96/src/vs/platform/theme/common/tokenClassificationRegistry.ts#L541-L582
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.
As per conversation in microsoft/vscode#145312 we could make these more specific, e.g. by appending .hcl
to all scopes, but I'm not sure what is the best long-term strategy for token/scope "parity", so I just mostly mimicked the existing behaviours for semantic tokens for now.
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 and works as expected; so much room for possibilities now!
Just a minor style suggestion, but feel free to ignore and merge anyway.
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
The feature is effectively gated on the server capability, so client and server part can be released independently. Our extension would just continue using the fallback token types and that one modifier until/unless the server claims support of the custom ones.
Related PRs:
terraform-ls
: internal/lsp: Provide (opt-in) custom semantic tokens & modifier terraform-ls#833hcl-lang
: Support custom modifiers viaBlockSchema
&LabelSchema
hcl-lang#106terraform-schema
: schema: Add custom token modifiers terraform-schema#99Follow-up issues:
superType
doesn't work as fallback withinsemanticTokenTypes
- i.e.semanticTokenScopes
are requiredPossible UX
I may not have picked the best colour combinations, but the point I'm trying to make here is not the colours, but the ability to assign colours to the relevant constructs in a meaningful way.
Background
This enables the wider ecosystem of themes by providing custom token types & modifiers for highlighting in a way, which:
Language server is mainly responsible for associating all the custom token types and modifiers to the relevant pieces of code, assuming that the client communicates via client capabilities that it supports these custom types & modifiers. This allows the server to still provide sensible semantic highlighting to clients which do not support these custom types & modifiers.
We could use these types & modifiers in our static grammar too, but I have not explored that possibility too deeply.
so given the above I'm leaning more towards keeping this mostly driven by the language server, at least for now.
The token types and modifiers are prefixed because apparently this is a global namespace and we don't want to end up in a conflict with any other extension.
microsoft/vscode#103097