Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add textDocument/inlayHints #1249
Add textDocument/inlayHints #1249
Changes from 1 commit
a00161e
99d150d
f414076
6e6379b
08804d4
84d0fe3
c62fb01
f82e51b
d55733d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are those categories necessary for a 1st iteration? I don't see categories in "Hover" nor "CodeLens" for instance. I think a 1st iteration should take place without them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, good question. These are present in both VSCode's proposed API and rust-analyzer (though Rust's categories have some overlap with classes, too)
I see two main purposes:
I find the configurability argument quite compelling, as this is a highly invasive/visible feature.
Categories don't add a lot of spec complexity, and are optional for both client & server.
So I'm inclined to include them but not wedded to it, definitely want to hear more opinions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The thing is that to properly handle it, it would require tools to dynamically generate some page which lists the available categories in the LS and allow to enable/disable some of them. The LS would require to allow more introspective than usual; categories/kinds are usually hardcoded pre-identified enums of things shared by many languages more than dynamically filled placeholders for LS-specific stuff.
If we need to distinguish different kinds, I think it's better to do it just like Completion and other do: the protocol defines a static list of supported kinds; that clients can then code a behavior for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that was definitely the intention here: clients could show fixed checkboxes for Params/Types/Other.
(In fact I thought about using classes for configuration, but it seems too complicated for exactly the reason you mention)
The reason to allow other values is just to make protocol versioning/extensions easier:
It's possible different servers use the same non-standard category name to mean different things, I don't think this is a large risk.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I've tweaked some comments and such, trying to make this clearer)