Skip to content
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

No longer possible to insert inlay hints #15604

Closed
mikenunan opened this issue Sep 13, 2023 · 5 comments · Fixed by #15635
Closed

No longer possible to insert inlay hints #15604

mikenunan opened this issue Sep 13, 2023 · 5 comments · Fixed by #15635
Labels
A-inlay-hints inlay/inline hints C-bug Category: bug

Comments

@mikenunan
Copy link

Prior to the latest release 2 days ago (0.3.1657 versus 0.3.1649 in Marketplace, or tags 2023-09-11 vs 2023-09-04), inlay hints carried a "Double-click to insert" capability in VSCode:

image

I recognise this had an issue anyway as the default gesture conflicts with code's hardwired ctrl+click mapping for go to definition, but the feature was not entirely unusable. The gesture for the inlay insert however is assignable I believe, so could we have the feature back with a non-conflicting binding possibly?

@mikenunan mikenunan added the C-bug Category: bug label Sep 13, 2023
@Veykril
Copy link
Member

Veykril commented Sep 13, 2023

Probably regressed with #15522

@Veykril Veykril added the A-inlay-hints inlay/inline hints label Sep 13, 2023
@SomeoneToIgnore
Copy link
Contributor

SomeoneToIgnore commented Sep 14, 2023

I can confirm that it's due to resolution, if we stop resolving edits it gets back to normal, this patch fixes it:

diff --git a/crates/rust-analyzer/src/lsp/to_proto.rs b/crates/rust-analyzer/src/lsp/to_proto.rs
index 23074493a..6943c10ec 100644
--- a/crates/rust-analyzer/src/lsp/to_proto.rs
+++ b/crates/rust-analyzer/src/lsp/to_proto.rs
@@ -446,7 +446,7 @@ pub(crate) fn inlay_hint(
     let needs_resolve = inlay_hint.needs_resolve;
     let (label, tooltip, mut something_to_resolve) =
         inlay_hint_label(snap, fields_to_resolve, needs_resolve, inlay_hint.label)?;
-    let text_edits = if needs_resolve && fields_to_resolve.resolve_text_edits {
+    let text_edits = if false && needs_resolve && fields_to_resolve.resolve_text_edits {
         something_to_resolve |= inlay_hint.text_edit.is_some();
         None
     } else {

Now the question is — why VSCode cannot handle resolved edits? It does send us textEdits in the client hint resolve capabilities

 resolve_support: Some(
            InlayHintResolveClientCapabilities {
                properties: [
                    "tooltip",
                    "textEdits",
                    "label.tooltip",
                    "label.location",
                    "label.command",
                ],
            },
        ),

and edits json that r-a sends back does not contain anything that might get obsolete (in the context of hint resolution, where we just hover on the established hint) :

"textEdits":[{"range":{"start":{"line":3,"character":10},"end":{"line":3,"character":10}},"newText":": Vec<String>"}]

So I do not see what exactly r-a does wrong, could it be VSCode bug? Would be nice to follow and discuss with them I presume.
As a workaround, we can introduce extra settings to ignore certain client hint resolve capabilities, but does it make any sense?
Are there better alternatives to solve this?

@Veykril
Copy link
Member

Veykril commented Sep 14, 2023

Might very well be a vscode bug. To work around we can just check if the client is vscode instead of adding a setting I think. The LSP initialization should name the client (at least VSCode names itself)

@mikenunan
Copy link
Author

Thanks to you all here for picking this up. Any thoughts around the gesture conflict and whether it can be fixed? Or should I open a separate issue for that once the feature is working again perhaps?

@SomeoneToIgnore
Copy link
Contributor

Hey @mikenunan , sorry, I have no good understanding of gesture conflict, but can stub the hint resolution the proposed way: #15635

Double click starts to work after the PR lands, so technically the issue would be closed 🙂
I would propose to create a separate issue for the conflict itself, if you think it will break eventually, and I am still not sure I follow why would double click stop working later, so more context inside the new issue would be great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-inlay-hints inlay/inline hints C-bug Category: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants