helix
helix copied to clipboard
bracket matching works incorrectly sometimes
Summary
For example in this example, with the cursor on either one of the parentheses in Some(thing)
, the matching one is not highlighted and hitting mm
doesn't take you to the matching one, instead it takes you to the bracket on line 6.
fn main() {
let opt: Option<()> = None;
match opt {
Some(thing) => {},
None => {}
}
}
Reproduction Steps
I tried this:
-
hx
- open the rust file with the example code from above, make sure the language is set to rust
- put the cursor on either one of the parentheses in
Some(thing)
- hit
mm
I expected this to happen: The matching parenthesis is highlighted in step 3, and moved to in step 4
Instead, this happened: Nothing is highlighted in step 3, and the bracket on line 6 is gone to in step 4
Helix log
~/.cache/helix/helix.log
please provide a copy of `~/.cache/helix/helix.log` here if possible, you may need to redact some of the lines
Platform
Linux
Terminal Emulator
Kitty
Helix Version
22.05-376-g6b84344e
This appears to be because the bracket matching expects matching brackets to be the border of a tree sitter node, but in this case Some
, (
, thing
and )
are all children of the same node, with no separate parent for (thing)
.
This is unlike the case of
let a = Some(5);
where (5)
are grouped separately under an arguments
node.
Is this an issue/bug in tree-sitter-rust (that they should group the (thing)
in the above example), or should we change the matching bracket code on our end?
If matching brackets within a node is possible without behavior regressions, that'd be preferrable. tree-sitter-rust actually already exposes a node just for the (5)
part and the parens are children of that
(call_expression
function: (identifier) ; "Some"
arguments:
(arguments ; "(5)"
(integer_literal))) ; "5"
So I'm not sure there's a change in tree-sitter-rust we can make to improve the situation.
tree-sitter-rust actually already exposes a node just for the (5) part and the parens are children of that
That's correct, but the problem is that in the case of match arms: Some
, (
, thing
and )
, are all siblings, with no node for just (thing)
. I do think tree-sitter rust could be changed, but it's also possible to fix this within helix (I'll work on this).
Oh yeah in match
s they are all under one node. We could fork tree-sitter-rust and ensure that tuple_struct_pattern
uses an arguments
node as a child to fix this case. I'm worried that other grammars are also inconsistent about having named nodes for their pairs. So I think it's best to improve this within helix if we can 👍
Also facing this issue with Zig:
const std = @import("std");
const StringHashMap = std.StringHashMap;
pub fn MyStruct() type {
return struct {
const Self = @This();
map: StringHashMap,
pub fn init() Self {
return Self{
.map = StringHashMap([]const u8).init(),
};
}
}; // <-- 'mm' here
} // <-- jumps here
The 2nd last }
jumps to the last }
.
Should this be a separate bug report?
I get wrong bracket matching with the following :lang html
documents:
<!-- () -->
<!-- ) -->
<!-- ( -->
The comments may contain more content. (Makes no difference.) This is a minimal reproduction.
In each instance, all matching brackets to (
or )
are mistakenly found to be >
.
Here is how tokyonight
highlights >
as matching bracket to (
and )
:
Note that the matching bracket to >
is (correctly) <
:
Same issue in C++ with macros:
#define TEST(A, B) ((A) + (B))
None of the parenthesis in ((A) + (B))
are highlighted.
#define TEST(A, B) ((A) + (B))
I think that's a separate issue. ma)
works for these cases just mm
and bracket highlights don't
I know this is not the perfect solution, but it would alleviate glaring cases like the one presented in this issue, where you are on top of the bracket: https://github.com/helix-editor/helix/pull/7238 I see it more as a trade-off to improve experience to most users with minimal required work.
Same issue in C++ with macros:
#define TEST(A, B) ((A) + (B))
None of the parenthesis in
((A) + (B))
are highlighted.
this is a pretty big edge-case. ) was able to handle everything else through TS bet C preprocessor tokens are just plaintext blobs. I handled that case with a super restrictive plaintext fallback. That case could be handled with TS too by injecting c into c preprocessor nodes the way nvim does but that feels hacky...