-
-
Notifications
You must be signed in to change notification settings - Fork 336
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
Inlay Hints for Control Flow Capture Values #1512
Conversation
No guarantees that this is the best way, or even the right way diff --git a/src/features/inlay_hints.zig b/src/features/inlay_hints.zig
index c3f9151..2431a34 100644
--- a/src/features/inlay_hints.zig
+++ b/src/features/inlay_hints.zig
@@ -315,7 +315,13 @@ fn writeForCaptureHint(builder: *Builder, for_node: Ast.Node.Index) !void {
defer tracy_zone.end();
const hint = builder.handle.tree.fullFor(for_node) orelse return;
- try appendTypeHintString(builder, hint.payload_token, try typeStrOfToken(builder, hint.payload_token) orelse return);
+ const token_tags = builder.handle.tree.tokens.items(.tag);
+ var token_index: u32 = hint.payload_token;
+ while (token_tags[token_index] != .pipe) : (token_index += 1) {
+ if (token_tags[token_index] == .comma) continue;
+ if (token_tags[token_index] == .asterisk) // Prepend `*` to the inlay hint
+ try appendTypeHintString(builder, token_index, try typeStrOfToken(builder, token_index) orelse return);
+ }
}
fn writeWhileCaptureHint(builder: *Builder, while_node: Ast.Node.Index) !void { |
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.
Techatrix is correct
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.
Other than the type needing to be future-proofed this looks ready.
Some subjective remarks:
- I wish zig itself wasn't using
node
<->node_index
interchangeably - Ditto
token
<->token_index
hint
isn't a good fit for the name of the result of fns that return (full)X node- These are so short, they could've been part of the
switch
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.
There is a missing test case for switch case captures and a test to see if for("",0..) |_, here<usize>|
works.
I am also wondering if there is any need to add a new config option. Shouldn't inlay_hints_show_variable_declaration
and inlay_hints_show_capture_variables
be merged into a single option since they both are type hints or is there interest in having only one of them enabled? Keep in mind that something like #1531 may even add another option to the mix.
When opting for inlay hints for variables, I suppose you'd want it on all the variables. |
Adds inlay hints hints for the capture values of the control flow statements (



if
,while
,for
,switch
).Fixed multiple capture values not working
