Skip to content

Some improvements to hover text#2277

Merged
Techatrix merged 2 commits into
zigtools:masterfrom
FnControlOption:hover-go-to
May 10, 2025
Merged

Some improvements to hover text#2277
Techatrix merged 2 commits into
zigtools:masterfrom
FnControlOption:hover-go-to

Conversation

@FnControlOption

Copy link
Copy Markdown
Contributor
  • Turned off parameter names when printing function types, which is consistent with how they're normally printed via std.fmt
  • Added the resolved type (in the parentheses) of a function when hovering over it, which makes the "Go to ..." part less confusing
    Screenshot 2025-04-23 at 10 08 01 AM
  • Merged Analyser.addReferencedTypes with Type.format, which has pretty much the same logic but also handles generic types
    Screenshot 2025-04-23 at 10 08 36 AM

@FnControlOption FnControlOption force-pushed the hover-go-to branch 5 times, most recently from 91bcea2 to 6b4fc40 Compare April 29, 2025 21:02
@FnControlOption FnControlOption force-pushed the hover-go-to branch 2 times, most recently from 382f6b0 to c0fa5ba Compare May 2, 2025 16:53
Comment thread src/analysis.zig Outdated
Comment thread src/features/hover.zig
@Techatrix

Copy link
Copy Markdown
Member

Why has this been converted into a draft? Is it waiting for #2296 perhaps?

@FnControlOption

Copy link
Copy Markdown
Contributor Author

Is it waiting for #2296 perhaps?

Yes, but we can merge this one first if you prefer

@FnControlOption FnControlOption marked this pull request as ready for review May 6, 2025 19:25
@FnControlOption FnControlOption marked this pull request as draft May 8, 2025 00:29
Comment thread tests/lsp_features/hover.zig Outdated
\\) void
\\```
\\```zig
\\(unknown)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have another PR in the works: FnControlOption/zls@8d5878bb (Will open after this one is merged)

@FnControlOption FnControlOption marked this pull request as ready for review May 8, 2025 02:06

@Techatrix Techatrix left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure how I feel about bringing back the resolved function type after it has been removed in #1377. I suppose it a matter of personal preference about how much information you would like to see in the hover popup.

I have recently found a possible solution to this problem that could become useful in the future. I will follow up about it in a new issue.

@Techatrix Techatrix merged commit d697a73 into zigtools:master May 10, 2025
6 checks passed
@FnControlOption FnControlOption deleted the hover-go-to branch May 11, 2025 04:46
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.

2 participants