-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Upstream inlay hints #11445
Upstream inlay hints #11445
Conversation
Huh, surprisingly, I've been doing similar things today. Does not work for me too: the hint provider is being called, but the TRACE logs in the extension host show me that 0 hints are actually emitted after that call. I'm not sure why that happens exactly, but will look closer this week. |
I've implemented that in my PR, you have to specify a certain preview feature since the old "enable preview api" setting does nothing and is deprecated.
That could be the case actually, I'll check it when I have time, thanks for the pointer. I'll continue my experiments in my branch, if you don't mind 🙂 |
It's not that. The API got stabilized today, so the proposed .d.ts is no longer there. |
I really hope that vscode has improved their styling of inlay hints, because the ones provided by the TS language server are super ugly compared to those provided via r-a's vscode extension... |
I am curious why this isn't working for you. Let me know if you have questions and something simple for my to try. Also, todays, insiders should have the API available without proposed guard. That makes things a little simpler |
@jrieken thanks! I posted yesterday on microsoft/vscode#129528, and I didn't manage to figure it out. Unfortunately, Insiders crashes on start-up in SQLite for me -- I couldn't find a bug for it, but I didn't report it yet. I know that a version from maybe a couple of weeks ago was working. |
Saw your comment on the other issue, replied inline (microsoft/vscode#129528 (comment)). Interesting finds, this fix is near :-) |
Thanks, @jrieken, I assumed it was something like this when I noticed that even the Well, I must be passing the wrong position anyway 😅 : Better screenshot: |
Nice. Glad to see this working (no matter the true position ;-)). Fyi - I have just pushed microsoft/vscode@0fd15bb which relaxes how the base types work |
@jrieken , a few questions and notes that I have:
that file contains all our client code that uses the proposed API. I will record a set of videos to show the caret and update cases. |
Nb. these actually work fine for me: What bothers me more is that they're usually not very well aligned with the code.
This is actually interesting, and it behaves somewhat differently depending on the direction you're moving in. |
@lnicola , have you tried opening more than one project? For me, only a single project now displays the hints, if I do |
Opening other projects with File / Open Recent works fine for me. |
carets_and_hints.movIn this PR's code, I've added a logging event just before the Here's what it showing for any hint update:
It's always 1-2 seconds of latency between the event firing and the
|
@jrieken , a few questions and notes that I have:
You actually cannot change the label, only tooltip, command, and location can be added/changed later. This is for a stable UI and extensions should provide hints with the final label.
/cc @dbaeumer Likely next milestone. It's winter and everyone is skiing ❄️
There is microsoft/vscode#127703 for more explicit stops when using arrow keys, clicking should add stops already. Looping in @hediet for details.
We artificially delay updates while typing - before that things were moving too much (see microsoft/vscode#133730). This of course can be tweaks further. With trace logging enabled, the window log will show the actual delay (it's a dynamic value based on the time taken, tho never under 25ms). It should look like so |
Thank you, good to be aware of this; might be worth mentioning in the documents.
Hints appear fast when scrolling indeed, but feels like different people could get used to different debouncing speeds, might be good to leave it configurable. Otherwise, feels really nice and code saving, thank you for having those. |
@jrieken I think I personally would either like to have an option to make the debouncing faster or at least have it indicate inlay hints which are outdated in some way until they are updated. Or maybe it could keep the size of the inlay hint identical for a bit and only change the contents, so you get empty space if it gets shorter and an ellipsis when it would overflow. |
I've played a bit and personally disliked the padding-less approach after a while, ergo bringing the padding back. I guess, we might want to add it via the settings eventually? Not sure if anybody wants that form though, due to odd alignment of the hints. I've also tried to emulate the dynamic hints and found the tooltips behaving a bit odd:
tooltips.mov |
Agree. I will push a change today that adds the underline on hover which become an active link when cmd is pressed (similar to links in the editor) Screen.Recording.2022-02-22.at.11.08.53.mov |
The inlay hint has been finalized in the latest vscode, maybe we could land it in the rust-analyzer now. |
b670693
to
26d2e88
Compare
Can we merge this to give it some time to bake? |
@lnicola, isn't it necessary to update the experimental capabilities as well? Thanks. |
bors r+ |
Probably I'm late here, but why do these new inlay hints not contain colons? It is very distracting to distinguish between a hint and code even with their different styling options. Or, should I open a separate issue for this? |
@RagibHasin the colons were added back in #11658. |
Closes rust-lang#6883 This functionality was changed as of rust-lang#11445 and now can be customized using native VSCode settings instead of `rust-analyzer`-specific ones.
InlayKind::ChainingHint => lsp_ext::InlayKind::ChainingHint, | ||
InlayKind::ParameterHint => Some(lsp_ext::InlayHintKind::PARAMETER), | ||
InlayKind::TypeHint => Some(lsp_ext::InlayHintKind::TYPE), | ||
InlayKind::ChainingHint => None, |
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.
why do chaining hints not have their own kind? (CC)
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.
Most likely cause the proposal only defines type and parameter hint kinds.
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.
Yes, here's a current corresponding definition on VSCode side: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/597a8ab3638d98ab8bfdbb07a6d6afec1dd45ab4/types/vscode/index.d.ts#L4581-L4596
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 suppose a chaining hint is a kind of type hint?
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.
Could be.
If we believe their docs, it should affect the appearance only(?)
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/597a8ab3638d98ab8bfdbb07a6d6afec1dd45ab4/types/vscode/index.d.ts#L4671-L4674
11708: Update manual for inlay hint customization r=Veykril a=ian-h-chamberlain Related to #6405, #6883 but not sure if they should be closed or not as this is just a documentation update. This functionality was changed as of #11445 and now can be customized using native VSCode settings instead of `rust-analyzer`-specific ones. Co-authored-by: Ian Chamberlain <ian-h-chamberlain@users.noreply.github.com>
11720: fix: Mark chaining hints as types r=SomeoneToIgnore a=lnicola CC #11445 (comment) Co-authored-by: Laurențiu Nicola <lnicola@dend.ro>
Closes rust-lang#6883 This functionality was changed as of rust-lang#11445 and now can be customized using native VSCode settings instead of `rust-analyzer`-specific ones.
424: New features to LSP r=ice1000 a=imkiva this PR upgraded LSP4j to 0.14 with inlay hints support; added some features to the LSP backend: - Inlay type hints for bind patterns (feel free to suggest more) - CodeLens of each definition showing `%d usages` - Code folding for definitions that occupy 3 or more LOC. - Search Everywhere in VSCode (symbols only since VSC does not ask more from backend) Try it: https://github.com/aya-prover/aya-vscode/suites/6895435995/artifacts/267446633 ## See also Language Server Protocol has recently upgraded to 3.17, and VSCode stabilized their inlay hints APIs - https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_inlayHint - rust-lang/rust-analyzer#11445 - microsoft/vscode-languageserver-node#495 - aya-prover/aya-vscode#21 Co-authored-by: imkiva <imkiva@islovely.icu>
Closes #2797
Closes #3394 (since now resolve the hints for the range given only, not for the whole document. We don't actually resolve anything due to hard requirement on label being immutable. Any further heavy actions could go to the
resolve
method that's now available via the official Code API for hints)Based on @SomeoneToIgnore's branch, with a couple of updates:
.d.ts
no longer works, but you can get it manually from https://raw.githubusercontent.com/microsoft/vscode/release/1.64/src/vscode-dts/vscode.proposed.inlayHints.d.ts--enable-proposed-api matklad.rust-analyzer
InlayHintKind
needs to be serialized as a number, not string