Skip to content

Comments

Implement textDocument/inlayHint#559

Merged
SuperAuguste merged 4 commits intozigtools:masterfrom
Techatrix:master
Jul 24, 2022
Merged

Implement textDocument/inlayHint#559
SuperAuguste merged 4 commits intozigtools:masterfrom
Techatrix:master

Conversation

@Techatrix
Copy link
Member

@Techatrix Techatrix commented Jul 23, 2022

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

SuperAuguste commented Jul 23, 2022

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

SuperAuguste commented Jul 24, 2022

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

@SuperAuguste
Copy link
Member

SuperAuguste commented Jul 24, 2022

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

Techatrix commented Jul 24, 2022

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

SuperAuguste commented Jul 24, 2022

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

SuperAuguste commented Jul 24, 2022

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

Techatrix commented Aug 21, 2022

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