feat: Add hover for structs #1916
Conversation
|
Hover over pub const blah = struct {
const str = "something";
};backtrace |
|
Oh wow, I completely blanked on covering that case. I'll be able to take another look later tonight. Sorry about that! |
|
I reworked the second case to avoid the crash and added a "catchall" case for variable declarations that aren't full container decls.
Given that the number of different nodes that can get caught in the " const EdgeCases = struct {
const str = "something";
const s = S{
.fld = 0,
};
};...shows up in the hover window using this approach:
I'm not sure how to better handle such a general parsing problem without needlessly reinventing the wheel. Maybe |
|
Thank you for working on this ! My opinion: field: struct {
...
},where surely no one would put more than 5 fields and absolutely no decls. To me, the perfect balance would be // Could be in any file
const T = struct {
inner: u8,
pub const default: T = .{ .inner = 1 };
const Self = @This();
pub fn inc(self: Self) void {
self.inner += 1;
}
};
test {
// or as fn param, ie v: T
const v = T.default; // Hover over T, hovering over `default` would still show `pub const default: T = .{ .inner = 1 };`
// much code here
_ = v;
}Result (only fields and pub decls, ie only what's available at the point of the var decl) : const T = struct {
inner: u8,
pub const default: T
pub fn inc(self: Self) void
};This gives me a general idea of what |
|
I spent some more time with this and worked out how to get utilize some pieces of I also wanted to mention that I currently have an extra empty line between separate decls, e.g. the following code snippet const EdgeCases = struct {
const str = "something";
const s = S{
.fld = 0,
};
};has the following hover window (with the extra empty line in between
I think this helps with readability, but I'd be fine removing said extra line if that's preferred :) |
|
@llogick Funny timing! Sorry I didn't see your comment before pushing again. I'll take another look at this tomorrow :) |
|
Alright I added support for functions, changed the existing code so that only pub decls show up, and removed the extra space between decls (there is still an empty line going from a field to a decl). Here's what llogick's example looks like with the current code. I think it might be worth it to keep default values (e.g. the
|
WillLillis
force-pushed
the
hover_struct
branch
3 times, most recently
from
cce9fad
to
cf2edd7
Compare
|
Would it be possible to show |
I like this idea, but don't have a great idea of how I'd implement it. I'll spend some time on it this weekend. :) Related: #1677 |
WillLillis
force-pushed
the
hover_struct
branch
2 times, most recently
from
2f8b76a
to
8470cb3
Compare
There was a problem hiding this comment.
Dealing with formatted code in ZLS is a complicated problem and we currently do not have the right tools to deal with it. std,zig.Ast.render can't be used because it can easily crash on code with syntax errors. Also, don't use tabs.
I have an alternative solution, just use the source code as-is and then remove extra indentation.
diff --git a/src/analysis.zig b/src/analysis.zig
index f2c0761f..dc794cdc 100644
--- a/src/analysis.zig
+++ b/src/analysis.zig
@@ -398,7 +398,7 @@ pub fn firstParamIs(
}
pub fn getVariableSignature(
- allocator: std.mem.Allocator,
+ arena: std.mem.Allocator,
tree: Ast,
var_decl: Ast.full.VarDecl,
include_name: bool,
@@ -421,7 +421,7 @@ pub fn getVariableSignature(
const end_token = switch (node_tags[init_node]) {
.merge_error_sets => {
if (!include_name) return "error";
- return try std.fmt.allocPrint(allocator, "{s} error", .{
+ return try std.fmt.allocPrint(arena, "{s} error", .{
offsets.tokensToSlice(tree, start_token, tree.firstToken(init_node) - 1),
});
},
@@ -458,6 +458,32 @@ pub fn getVariableSignature(
offset += 1;
}
+ if (container_decl.ast.members.len != 0) container_fields: {
+ var first_field: ?Ast.Node.Index = null;
+ var last_field: ?Ast.Node.Index = null;
+
+ for (container_decl.ast.members) |member| {
+ switch (tree.nodes.items(.tag)[member]) {
+ .container_field_init, .container_field_align, .container_field => {},
+ else => continue,
+ }
+ if (first_field == null) first_field = member;
+ last_field = member;
+ }
+ if (first_field == null or last_field == null) break :container_fields;
+
+ const first_field_line_start = offsets.lineLocUntilIndex(tree.source, offsets.tokenToIndex(tree, tree.firstToken(first_field.?))).start;
+ const fields_source_indented = tree.source[first_field_line_start..offsets.tokenToLoc(tree, ast.lastToken(tree, last_field.?)).end];
+ const fields_source = try trimCommonIndentation(arena, fields_source_indented, 4);
+
+ return try std.mem.concat(arena, u8, &.{
+ offsets.tokensToSlice(tree, start_token, token + offset),
+ " {\n",
+ fields_source,
+ "\n}",
+ });
+ }
+
break :blk token + offset;
},
else => ast.lastToken(tree, init_node),
@@ -466,6 +492,68 @@ pub fn getVariableSignature(
return offsets.tokensToSlice(tree, start_token, end_token);
}
+fn trimCommonIndentation(allocator: std.mem.Allocator, str: []const u8, preserved_indentation_amount: usize) error{OutOfMemory}![]u8 {
+ var line_it = std.mem.splitScalar(u8, str, '\n');
+
+ var non_empty_lines: usize = 0;
+ var min_indentation: ?usize = null;
+ while (line_it.next()) |line| {
+ if (line.len == 0) continue;
+ const indentation = for (line, 0..) |c, count| {
+ if (!std.ascii.isWhitespace(c)) break count;
+ } else line.len;
+ min_indentation = if (min_indentation) |old| @min(old, indentation) else indentation;
+ non_empty_lines += 1;
+ }
+
+ var common_indent = min_indentation orelse return try allocator.dupe(u8, str);
+ common_indent -|= preserved_indentation_amount;
+ if (common_indent == 0) return try allocator.dupe(u8, str);
+
+ const capacity = str.len - non_empty_lines * common_indent;
+ var output = try std.ArrayListUnmanaged(u8).initCapacity(allocator, capacity);
+ std.debug.assert(capacity == output.capacity);
+ errdefer @compileError("error would leak here");
+
+ line_it = std.mem.splitScalar(u8, str, '\n');
+ var is_first_line = true;
+ while (line_it.next()) |line| {
+ if (!is_first_line) output.appendAssumeCapacity('\n');
+ if (line.len != 0) {
+ output.appendSliceAssumeCapacity(line[common_indent..]);
+ }
+ is_first_line = false;
+ }
+
+ std.debug.assert(output.items.len == output.capacity);
+ return output.items;
+}
+
+test trimCommonIndentation {
+ const cases = [_]struct { []const u8, []const u8, usize }{
+ .{ "", "", 0 },
+ .{ "\n", "\n", 0 },
+ .{ "foo", "foo", 0 },
+ .{ "foo", " foo", 0 },
+ .{ "foo ", " foo ", 0 },
+ .{ "foo\nbar", " foo\n bar", 0 },
+ .{ "foo\nbar\n", " foo\n bar\n", 0 },
+ .{ " foo\nbar", " foo\n bar", 0 },
+ .{ "foo\n bar", " foo\n bar", 0 },
+ .{ " foo\n\nbar", " foo\n\n bar", 0 },
+
+ .{ " foo\n bar", " foo\n bar", 2 },
+ .{ " foo\n bar", " foo\n bar", 4 },
+ .{ " foo\n bar", " foo\n bar", 8 },
+ };
+
+ for (cases) |case| {
+ const actual = try trimCommonIndentation(std.testing.allocator, case[1], case[2]);
+ defer std.testing.allocator.free(actual);
+ try std.testing.expectEqualStrings(case[0], actual);
+ }
+}
+
pub fn getContainerFieldSignature(tree: Ast, field: Ast.full.ContainerField) ?[]const u8 {
if (field.ast.type_expr == 0) return null;
const end_node = if (field.ast.value_expr != 0) field.ast.value_expr else if (field.ast.align_expr != 0) field.ast.align_expr else field.ast.type_expr;
Nice! I reworked it a bit to also show |
fix crash, add catch-all case Better fix for default values Only show 'pub' members, add support for functions, update existing tests add hover test for public fn, private decls
Add missing 'else' case
This PR adds hover support for structs, enums, and unions without displaying member functions, as discussed in #1568. Going off of this comment from this issue, I also changed the final layout so that doc comments appear on top rather than the bottom of the hover window. If this needs to be discussed further/ moved into a separate PR I'm happy to do so!
A few examples:
cc: @llogick
Closes #1568