-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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(lang): Update Nix grammar & improve queries #2472
Conversation
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.
Helix uses custom scopes (see here), so some of the scopes here need to be renamed. For example: @property
becomes @variable.other.member
. The changes for booleans and numbers can be reverted except that it looks like integer
and float
nodes have been renamed to integer_expression
and float_expression
. It looks like most of the changes in the grammar came from renaming nodes to have a _expression
suffix.
@the-mikedavis, I believe I addressed all your comments with a force push. |
530fcd8
to
0ca07a9
Compare
[ | ||
"}" | ||
"]" | ||
] @outdent |
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.
We should probably keep these unless the injects work better without them
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.
Not sure exactly why tbh, but I was getting extra indents with the previous definitions. These ones seem to work more accurately
6a19ab6
to
319a4a0
Compare
@the-mikedavis I think I was able to resolve everything. I just recently stumbled on this comment though: nix-community/tree-sitter-nix#13 (comment) and now I'm wondering if I should rebase on oxalica's fork to add the injections. Although what I'd really like to do is get those changes incooperated into upstream properly. I think I'll give this a shot later. Maybe we should hold off on merging for a bit just in case I'm successful? |
I think this is a good increment even without shell injections - that can always be a follow-up PR. Would you mind rebasing? |
I went ahead and force pushed a rebase, and added this back: diff --git a/runtime/queries/nix/highlights.scm b/runtime/queries/nix/highlights.scm
index d15fb9c2..a171a39b 100644
--- a/runtime/queries/nix/highlights.scm
+++ b/runtime/queries/nix/highlights.scm
@@ -72,9 +72,15 @@
(binary_expression
operator: _ @operator)
+(variable_expression (identifier) @variable)
+
(binding
attrpath: (attrpath (identifier)) @variable.other.member)
+(identifier) @variable.other.member
+
+(inherit_from attrs: (inherited_attrs attr: (identifier) @variable.other.member) )
+
[
";"
"." Without it, I experienced a regression in |
The
stanza is a good catch: it highlights variables in inherit-from statements: {
inherit (common) pkgs;
# ^ variable
# ^ variable.other.member
} The
stanza takes priority over the
on the last line, so let's remove the The
doesn't appear to have an effect because of the By path highlights did you mean like |
Update grammar and queiries to the latest upstream. Also add improved indents thanks to @oxalica.
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 great, thanks for working on this!
Thanks for the feedback. Also, just to confirm, it is #2484 that is the problem. After opening the same file several times I noticed sometimes paths are highlighted correcly, and sometimes they are just hightlighted as regular strings. |
Update grammar and queiries to the latest upstream. Also add improved indents thanks to @oxalica.