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

fix: set the right postfix snippets competion source range #17000

Merged
merged 1 commit into from Apr 3, 2024

Conversation

mrnossiom
Copy link
Contributor

Hi 👋,

Changed the completion item source_range to match the replaced text. Though in VS Code it may not be disturbing because the snippet is previewed in a box, but in Helix editor, it's previewed by applying the main text edit.

Before :
image

After :
image

Thanks

Changed the completion item source_range to match
the replaced text. Though in VS Code it may not be
disturbing because the snippet is previewed in a
box, but in Helix editor, it's previewed by applying
the main text edit.
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 2, 2024
@Veykril
Copy link
Member

Veykril commented Apr 3, 2024

Thanks!
@bors r+

@bors
Copy link
Collaborator

bors commented Apr 3, 2024

📌 Commit c5686c8 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Apr 3, 2024

⌛ Testing commit c5686c8 with merge 5b08b17...

@bors
Copy link
Collaborator

bors commented Apr 3, 2024

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 5b08b17 to master...

@bors bors merged commit 5b08b17 into rust-lang:master Apr 3, 2024
11 checks passed
@pascalkuthe
Copy link
Contributor

pascalkuthe commented Apr 5, 2024

thanks for working on this! I just wanted to say that this doesn't just fix the preview (which is just a visual thing) but also multicursor completions.

Additional text edits can't be mapped to multiple cursor (since they aren't associated with the cursor. The spec actually considers this use case. You don't want to repeat imports for each cursor for example).

I see that RA does this as a workaround for LSP only allowing a single edit to each completion item. I think that generally makes sense for associated edits like imports but I can't think of a practical case where multiple edits would be associated with each cursor. I suspect the RA completion edit could be brought closer to the lsp completion edit and avoid impedance mismatches like this bug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants