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

Completion resolve support with textEdit #4607

Open
3 tasks done
traviscross opened this issue Nov 11, 2024 · 9 comments
Open
3 tasks done

Completion resolve support with textEdit #4607

traviscross opened this issue Nov 11, 2024 · 9 comments
Labels

Comments

@traviscross
Copy link

traviscross commented Nov 11, 2024

Thank you for the bug report

  • I am using the latest version of lsp-mode related packages.
  • I checked FAQ and Troubleshooting sections
  • You may also try reproduce the issue using clean environment using the following command: M-x lsp-start-plain

Bug description

This PR in rust-analyzer triggered a set of behavior regressions when using Emacs and lsp-mode:

On the r-a side, there is discussion to the effect that lsp-mode (and other editors) may be less spec-conformant than desired. See these comments and related issues:

This issue is opened in the hope that, in connecting here the experts on each side, that some agreement and resolution may be reached.

cc @jcs090218

Steps to reproduce

N/A.

Expected behavior

N/A.

Which Language Server did you use?

lsp-rust

OS

Linux

Error callstack

No response

Anything else?

No response

@kiennq
Copy link
Member

kiennq commented Nov 11, 2024

lsp-mode already support, can you try to lsp-toggle-trace-io to see if the ‘completionItem/resolve’ is sent in your case? When I tried it already worked though

@traviscross
Copy link
Author

Thanks for having a look. After further analysis, the r-a developers decided this was in fact a bug in rust-analyzer:

@traviscross traviscross closed this as not planned Won't fix, can't repro, duplicate, stale Nov 11, 2024
@SomeoneToIgnore
Copy link

There seems to be another bogus issue created: rust-lang/rust-analyzer#18504
Which seems to be very related to the spec interpretation, based on #4591 OP:

Removing "documentation" + "detail" from
....

and

Can you try to move to the next item in the completion list to see if the function detail will be displayed for the previously selected item?

does not have an effect; details are not shown

change, that this part is not really resolved properly in lsp part of Emacs.

So, this issue is still technically relevant, but not correctly composed?

@traviscross
Copy link
Author

OK. Reopening on that basis.

@traviscross traviscross reopened this Nov 11, 2024
@SomeoneToIgnore
Copy link

SomeoneToIgnore commented Nov 11, 2024

To add on top, I think that the OP nails it: for now, one has to remove

"documentation" + "detail"

from the resolve capabilities to fix the things.

Before, r-a responded with both fields despite the capabilities, now it's more spec-compliant, which that causes issues on the client side.
Removing the corresponding resolve capabilities will restore the status quo, and more clever approach may be implemented later.

@kiennq
Copy link
Member

kiennq commented Nov 12, 2024

The completion resolve with extra edit is already supported. If you have some example where it doesn't work, I would like to try it to see why.
Here is the working case in my env
9e4b6112-48c8-457d-bdff-6d699e056691

@traviscross
Copy link
Author

traviscross commented Nov 12, 2024

Note there are two issues here:

The first was about automatically importing items. This was reported here:

And was then fixed on the r-a side here:

So this will work properly with r-a nightly (due to the fix) or in the Rust r-a distributed with Rust 1.82 (because that is before the change was introduced).

(If you want to see this problem, test with commit rust-lang/rust-analyzer@950bb83.)

The second item is about failing to display the details of method signatures. This is tracked on the lsp-mode side here:

And on the r-a side here:

This issue is present in r-a nightly and is not fixed by the other change. This is not present in the r-a distributed with Rust 1.82, so you'll need to test with nightly to see the problem.

Details for reproducing this result in a clean environment are described here:

rust-lang/rust-analyzer#18504 (comment)

It's been reported that eglot is not affected by this. I.e., that it continues to show full method signatures.

This issue is filed separately to track the general item about specification conformance potentially related to both issues that has been described by @SomeoneToIgnore above. Both these regressions were prompted by the same PR on the r-a side here:

@kiennq
Copy link
Member

kiennq commented Nov 12, 2024

The resolve for detail (signature) should be supported in #4610
Before that, if you enable company-auto-update-doc it should be able to show the detail on-demand, after the item is focused.

@wyuenho
Copy link
Contributor

wyuenho commented Dec 12, 2024

WRT to textEdit, rust-analyzer still returns textEdit from the response in textDocument/completion, both for stable and nightly. We can close this ticket and focus on #4591, perhaps file a bug on rust-analyzer, once there's an agreement over there, and can perhaps roll back #4610 as neither insertTextMode nor insertTextFormat need to be resolved judging by the current state of affairs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants