Resolve completions properly #18212
Merged
Resolve completions properly #18212
+35
−11
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cla-bot
bot
added
the
cla-signed
SomeoneToIgnore
force-pushed
the
kb/proper-completion-resolve
branch
from
03a2bac
to
db8fc7c
Compare
bors
added a commit
to rust-lang/rust-analyzer
that referenced
this pull request
Sep 30, 2024
internal: Send less data during `textDocument/completion` if possible Similar to #15522, stops sending extra data during `textDocument/completion` if that data was set in the client completions resolve capabilities, and sends those only during `completionItem/resolve` requests. Currently, rust-analyzer sends back all fields (including potentially huge docs) for every completion item which might get large. Same as the other one, this PR aims to keep the changes minimal and does not remove extra computations for such fields — instead, it just filters them out before sending to the client. The PR omits primitive, boolean and integer, types such as `deprecated`, `preselect`, `insertTextFormat`, `insertTextMode`, etc. AND `additionalTextEdits` — this one looks very dangerous to compute for each completion item (as the spec says we ought to if there's no corresponding resolve capabilities provided) due to the diff computations and the fact that this code had been in the resolution for some time. It would be good to resolve this lazily too, please let me know if it's ok to do. When tested with Zed which only defines `documentation` and `additionalTextEdits` in its client completion resolve capabilities, rust-analyzer starts to send almost 3 times less characters: Request: ```json {"jsonrpc":"2.0","id":104,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///Users/someonetoignore/work/rust-analyzer/crates/ide/src/inlay_hints.rs"},"position":{"line":90,"character":14},"context":{"triggerKind":1}}} ``` <img width="1338" alt="image" src="https://github.com/user-attachments/assets/104f19b5-7095-4fc1-b008-5d829623b2e2"> Before: 381944 characters [before.json](https://github.com/user-attachments/files/17092385/before.json) After: 140503 characters [after.json](https://github.com/user-attachments/files/17092386/after.json) After Zed's [patch](zed-industries/zed#18212) to enable all resolving possible: 84452 characters [after-after.json](https://github.com/user-attachments/files/17092755/after-after.json)
lnicola
pushed a commit
to lnicola/rust
that referenced
this pull request
Oct 8, 2024
…ykril internal: Send less data during `textDocument/completion` if possible Similar to rust-lang/rust-analyzer#15522, stops sending extra data during `textDocument/completion` if that data was set in the client completions resolve capabilities, and sends those only during `completionItem/resolve` requests. Currently, rust-analyzer sends back all fields (including potentially huge docs) for every completion item which might get large. Same as the other one, this PR aims to keep the changes minimal and does not remove extra computations for such fields — instead, it just filters them out before sending to the client. The PR omits primitive, boolean and integer, types such as `deprecated`, `preselect`, `insertTextFormat`, `insertTextMode`, etc. AND `additionalTextEdits` — this one looks very dangerous to compute for each completion item (as the spec says we ought to if there's no corresponding resolve capabilities provided) due to the diff computations and the fact that this code had been in the resolution for some time. It would be good to resolve this lazily too, please let me know if it's ok to do. When tested with Zed which only defines `documentation` and `additionalTextEdits` in its client completion resolve capabilities, rust-analyzer starts to send almost 3 times less characters: Request: ```json {"jsonrpc":"2.0","id":104,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///Users/someonetoignore/work/rust-analyzer/crates/ide/src/inlay_hints.rs"},"position":{"line":90,"character":14},"context":{"triggerKind":1}}} ``` <img width="1338" alt="image" src="https://github.com/user-attachments/assets/104f19b5-7095-4fc1-b008-5d829623b2e2"> Before: 381944 characters [before.json](https://github.com/user-attachments/files/17092385/before.json) After: 140503 characters [after.json](https://github.com/user-attachments/files/17092386/after.json) After Zed's [patch](zed-industries/zed#18212) to enable all resolving possible: 84452 characters [after-after.json](https://github.com/user-attachments/files/17092755/after-after.json)
noaccOS
pushed a commit
to noaccOS/zed
that referenced
this pull request
Oct 19, 2024
Related to rust-lang/rust-analyzer#18167 * Declare more completion item fields in the client completion resolve capabilities * Do resolve completions even if their docs are present * Instead, do not resolve completions that could not be resolved when handling the remote client resolve requests * Do replace the old lsp completion data with the resolved one Release Notes: - Improved completion resolve mechanism
github-actions bot
pushed a commit
to rust-lang/miri
that referenced
this pull request
Nov 9, 2024
internal: Send less data during `textDocument/completion` if possible Similar to rust-lang/rust-analyzer#15522, stops sending extra data during `textDocument/completion` if that data was set in the client completions resolve capabilities, and sends those only during `completionItem/resolve` requests. Currently, rust-analyzer sends back all fields (including potentially huge docs) for every completion item which might get large. Same as the other one, this PR aims to keep the changes minimal and does not remove extra computations for such fields — instead, it just filters them out before sending to the client. The PR omits primitive, boolean and integer, types such as `deprecated`, `preselect`, `insertTextFormat`, `insertTextMode`, etc. AND `additionalTextEdits` — this one looks very dangerous to compute for each completion item (as the spec says we ought to if there's no corresponding resolve capabilities provided) due to the diff computations and the fact that this code had been in the resolution for some time. It would be good to resolve this lazily too, please let me know if it's ok to do. When tested with Zed which only defines `documentation` and `additionalTextEdits` in its client completion resolve capabilities, rust-analyzer starts to send almost 3 times less characters: Request: ```json {"jsonrpc":"2.0","id":104,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///Users/someonetoignore/work/rust-analyzer/crates/ide/src/inlay_hints.rs"},"position":{"line":90,"character":14},"context":{"triggerKind":1}}} ``` <img width="1338" alt="image" src="https://github.com/user-attachments/assets/104f19b5-7095-4fc1-b008-5d829623b2e2"> Before: 381944 characters [before.json](https://github.com/user-attachments/files/17092385/before.json) After: 140503 characters [after.json](https://github.com/user-attachments/files/17092386/after.json) After Zed's [patch](zed-industries/zed#18212) to enable all resolving possible: 84452 characters [after-after.json](https://github.com/user-attachments/files/17092755/after-after.json)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Related to rust-lang/rust-analyzer#18167
Release Notes: