nvim-treesitter icon indicating copy to clipboard operation
nvim-treesitter copied to clipboard

fix(zig): type block indents

Open coderkalyan opened this issue 1 year ago • 7 comments

I was a bit confused with how the zig treesitter grammar works, since the existing query does have a boolean capture. However, the logic around reserved identifiers doesn't seem to cooperate. In any case, capturing the keywords directly should cover any existing cases, and as far as I understand, there aren't any contexts where the keywords don't refer to booleans in the zig spec.

Also added indents for the type {} "blocks" (struct, enum, etc).

coderkalyan avatar Sep 30 '24 05:09 coderkalyan

For the boolean change, do you have an example of when this might be desired? I don't really know zig at all but seems like in the (weird) case of, e.g., var true = 5, true should be an identifier still and not a boolean?

ribru17 avatar Sep 30 '24 17:09 ribru17

Hmm, perhaps a different solution is needed then. For the record, var true = 5 is not legal zig:

$ cat /tmp/var.zig 
pub fn main() void {
    const true = 123;
}
$ zig run /tmp/var.zig 
/tmp/var.zig:2:11: error: name shadows primitive 'true'
    const true = 123;
          ^~~~
/tmp/var.zig:2:11: note: consider using @"true" to disambiguate

But I'm not addressing some weird edge case - there might be something wrong with the underlying grammar, but currently I'm not getting any highlights for true and false using the new grammar: image

Maybe @amaanq knows why this is happening?

coderkalyan avatar Sep 30 '24 18:09 coderkalyan

Ah thx, looks like that should be (boolean) not (identifier)

ribru17 avatar Sep 30 '24 18:09 ribru17

Something like this should work:

$ cat -p /tmp/test.zig 
pub fn main() void {
    const a = true;
    const b = false;
}
$ tree-sitter generate && tree-sitter parse /tmp/test.zig
(source_file [0, 0] - [4, 0]
  (function_declaration [0, 0] - [3, 1]
    name: (identifier [0, 7] - [0, 11])
    (parameters [0, 11] - [0, 13])
    type: (builtin_type [0, 14] - [0, 18])
    body: (block [0, 19] - [3, 1]
      (variable_declaration [1, 4] - [1, 19]
        (identifier [1, 10] - [1, 11])
        (identifier [1, 14] - [1, 18]))
      (variable_declaration [2, 4] - [2, 20]
        (identifier [2, 10] - [2, 11])
        (identifier [2, 14] - [2, 19])))))
$ git stash pop
$ git diff grammar.js 
diff --git a/grammar.js b/grammar.js
index 4d57b8c..5fa83a2 100644
--- a/grammar.js
+++ b/grammar.js
@@ -65,7 +65,6 @@ module.exports = grammar({
     [$.comptime_type_expression, $.parameter],
 
     [$._reserved_identifier, $.primary_type_expression],
-    [$._reserved_identifier, $.boolean],
   ],
 
   extras: $ => [
@@ -75,6 +74,7 @@ module.exports = grammar({
 
   precedences: $ => [
     [$.container_field, $.type_expression],
+    [$.boolean, $._reserved_identifier],
   ],
 
   supertypes: $ => [
$ tree-sitter generate && tree-sitter parse /tmp/test.zig
(source_file [0, 0] - [4, 0]
  (function_declaration [0, 0] - [3, 1]
    name: (identifier [0, 7] - [0, 11])
    (parameters [0, 11] - [0, 13])
    type: (builtin_type [0, 14] - [0, 18])
    body: (block [0, 19] - [3, 1]
      (variable_declaration [1, 4] - [1, 19]
        (identifier [1, 10] - [1, 11])
        (boolean [1, 14] - [1, 18]))
      (variable_declaration [2, 4] - [2, 20]
        (identifier [2, 10] - [2, 11])
        (boolean [2, 14] - [2, 19])))))

Is this the right direction to go? Happy to submit a patch to the grammar based on your suggestions (and keep this PR as a indent only change).

coderkalyan avatar Sep 30 '24 18:09 coderkalyan

@ribru17 I updated this patch to include only the indent fixes. Added another indent fix so only the closing braces capture @indent.branch instead of opening ones, so that standalone blocks like these work correctly:

pub fn main() void {
    // outer
    {
        // inner
    }
}

I checked some other queries (like rust) and they seem to follow this pattern. Should be good for you to take another look!

coderkalyan avatar Oct 01 '24 02:10 coderkalyan

Thank you! Your grammar changes are probably good too, but going to wait for Amaan's expertise to confirm. As for the indent changes, would you mind adding a couple tests to show future code readers what was done (check out zig_spec.lua)? Then I think we can merge :)

ribru17 avatar Oct 01 '24 03:10 ribru17

Sorry for the delay, will plan to fix this up today or tomorrow.

coderkalyan avatar Oct 07 '24 19:10 coderkalyan

Closing as stale. A new PR (from a feature branch, not master!) would be welcome.

clason avatar Feb 25 '25 10:02 clason

Oh I completely forgot about this and just saw the notification! I'll set myself a reminder to submit a new PR.

coderkalyan avatar Feb 25 '25 23:02 coderkalyan