Skip to content

Implement textDocument/inlayHint #559

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jul 24, 2022
Merged

Implement textDocument/inlayHint #559

merged 4 commits into from
Jul 24, 2022

Conversation

Techatrix
Copy link
Member

The LSP Specification introduced inlay hints in version 3.17 Reference

This PR adds supports for parameter inlay hints for function calls including builtins.

This feature is disabled by default and must be enabled by settings enable_inlay_hints in zls.json or through zls config

If a function call can display hover information but can't show inlay hints its probably a bug, feel free to inform me.

Options

The following options are currently implemented:

Option Type Default value What it Does
inlay_hints_show_builtin bool true Enable inlay hints for builtin functions
inlay_hints_exclude_builtins [][]const u8 [] Don't show inlay hints for the given builtin functions. Builtins with one parameter are skipped automatically
inlay_hints_exclude_single_argument bool true Don't show inlay hints for single argument calls

Note that these options are located in src/inlay_hints.zig and are currently not part of zls config or zls.json.

I am open to suggestions for more options and better defaults.

Examples taken from VS Code



Hovering over a hint will show its type

Tested on

  • Platforms: Linux, Windows
  • Code Editors: VS Code
  • VS Code extensions: augusterame.zls-vscode and tiehuis.zig
  • Code bases: zig std lib, zls itself, gyro, zig-gamedev
  • zig compiler: 0.10.0-dev.3258+9734e643f

@SuperAuguste
Copy link
Member

Ok so this is absolutely awesome!! Heading to bed now but I'll look at this ASAP tomorrow! Thank you so much!

@SuperAuguste
Copy link
Member

I'll take care of the TODO expose options right now! :)

@SuperAuguste
Copy link
Member

Gotta figure out what's up with the CI, I'll try to unbrick it really quickly and then merge this!

@Techatrix
Copy link
Member Author

Aren't the config options meant for the end user?
Because i don't think inlay_hints_max_inline_children should be put among them. It is meant to reduce run times and doesn't affect the (visible) output.

@SuperAuguste
Copy link
Member

Aren't the config options meant for the end user? Because i don't think inlay_hints_max_inline_children should be put among them. It is meant to reduce run times and doesn't affect the (visible) output.

I couldn't quite figure out what it was doing so I added it to the config anyways, thanks for letting me know that this isn't meant to be configurable for end-users! I'll fix it up once I get tests passing.

@SuperAuguste
Copy link
Member

I couldn't salvage tests, I'll finalize the config and then work on fixing them in a separate PR; sorry for the trouble!

@SuperAuguste SuperAuguste merged commit 0ecdeee into zigtools:master Jul 24, 2022
@smokku
Copy link

smokku commented Aug 21, 2022

@Techatrix Thank you so much for this! :-)

Do you have plans for adding inlay hints for local const/var variables?

@Techatrix
Copy link
Member Author

I didn't implement them because right now they wouldn't work in many cases. You can check that by hovering over many symbols, if you see 'unknown' as the type then inlay hints wouldn't show a type as well. Once type resolution is improved we can work on inlay hints for types.

@leecannon leecannon mentioned this pull request Aug 23, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Techatrix @SuperAuguste @smokku