Skip to content
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

Hover: Add links to referenced types when possible #1281

Merged
merged 7 commits into from
Jun 29, 2023

Conversation

FnControlOption
Copy link
Contributor

@FnControlOption FnControlOption commented Jun 28, 2023

Inspired by rust-analyzer, this PR adds "Go to <type>" in hover pop-ups:

pointer function chain

Copy link
Member

@Techatrix Techatrix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, but I found some weird behavior when hovering over type declarations:

// example_file.zig
const SomeType = struct {};
const Self = @This();

If you hover on SomeType, you will get Go to *SomeType*
If you hover on Self, you will get Go to *example_file*

src/analysis.zig Outdated Show resolved Hide resolved
src/features/hover.zig Outdated Show resolved Hide resolved
src/analysis.zig Show resolved Hide resolved
@FnControlOption
Copy link
Contributor Author

FnControlOption commented Jun 28, 2023

Great work

Thanks, I appreciate it 😄

I found some weird behavior when hovering over type declarations:

// example_file.zig
const SomeType = struct {};
const Self = @This();

If you hover on SomeType, you will get Go to *SomeType* If you hover on Self, you will get Go to *example_file*

I have an idea for avoiding redundant "Go to" for SomeType, but for Self, I'm not sure if it's possible to improve that without some refactoring. Currently, type aliases get completely resolved to whatever's the underlying type. (In the Zig stdlib, this isn't a problem since AFAIK file-level structs are named the same as normal structs, e.g. Allocator.zig) I have some local changes that adds .type_function variant to Type.data. Maybe I'll also need to add a .type_alias variant too.

@Techatrix
Copy link
Member

If fixing the Self example is too complex, focusing just on SomeType should be fine, especially because that example is far more common to encounter.

Currently, type aliases get completely resolved to whatever's the underlying type. (In the Zig stdlib, this isn't a problem since AFAIK file-level structs are named the same as normal structs, e.g. Allocator.zig)

Maybe resolveVarDeclAlias is what you need. It will resolve aliases but not completely resolve its type.

I have some local changes that adds .type_function variant to Type.data.

functions in Type can already be represented with the Type.data.other but a more explicit case may be helpful.

Copy link
Member

@Techatrix Techatrix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything is in order except that you should use offsets.nodeToSlice instead of std.zig.Ast.getNodeSource because ZLS uses a custom lastToken function that is safe to use on ast with syntax errors.

src/features/hover.zig Outdated Show resolved Hide resolved
@Techatrix Techatrix merged commit c31e723 into zigtools:master Jun 29, 2023
4 checks passed
acristoffers pushed a commit to acristoffers/zls that referenced this pull request Jul 5, 2023
KoltPenny pushed a commit to KoltPenny/zls that referenced this pull request Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants