-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
feat: Add default configuration for KCL language server #13148
base: master
Are you sure you want to change the base?
Conversation
5e08292
to
7a86f22
Compare
Generally we don't accept language definitions that don't also have syntax highlighting. Language server configurations can go in https://github.com/helix-editor/helix/wiki/Language-Server-Configurations |
I came across nvim-treesitter/nvim-treesitter#7657 the other day as I was looking to upstream something to nvim-treesitter, so looks like there is a maintained Tree-sitter grammar you can use! https://github.com/kcl-lang/tree-sitter-kcl |
5fd2e21
to
5682776
Compare
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.
Some suggestions on how to improve the queries :)
runtime/queries/kcl/highlights.scm
Outdated
"and" | ||
"or" | ||
"not" | ||
"in" | ||
"is" |
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.
Some/all of these can be under keyword.operator
instead.
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 moved the ones commented here, or did you mean all of them under @operator
?
runtime/queries/kcl/highlights.scm
Outdated
|
||
(selector_expr | ||
(select_suffix | ||
(identifier) @property)) |
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.
(identifier) @property)) | |
(identifier) @property)) |
property
isn't an official scope for Helix, maybe variable.other.member
is fitting if this node matches the description of "Fields of composite data types (e.g. structs, unions)"?
Wow, awesome @uncenter - but this |
It's a little complicated because every Tree-sitter based editor does things differently. The original queries here (from the upstream repository) look like they are intended for usage in Neovim, whereas Helix uses some different scopes in its themes so we have to change it up a bit (no need to upstream these changes back since the changes will be specific to Helix). |
Co-authored-by: uncenter <[email protected]>
Co-authored-by: uncenter <[email protected]>
Co-authored-by: uncenter <[email protected]>
Co-authored-by: uncenter <[email protected]>
Co-authored-by: uncenter <[email protected]>
b0a17a6
to
490e964
Compare
Adds a default configuration for KCL's language server.
I'm not sure where else this needs to be added.
It looks like the
--health
argument operates dynamically enough.Where in the tests might a test for this go?
Should I add something to the release notes?
Fix #13147