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

internal: Defer structured snippet rendering to allow escaping snippet bits #15269

Merged

Conversation

DropDemBits
Copy link
Contributor

Since we know exactly where snippets are, we can transparently escape snippet bits to the exact text edits that need it, and not have to do it for anything other text edits.

Also will eventually fix #11006 once all assists are migrated. This comes as a side-effect of text edits that don't have snippets get marked as having no insert formatting at all.

Rendering of snippet edits is deferred to places using source change
This ensures that any assist using structured snippets won't
accidentally remove bits interpreted as snippet bits.
Also makes sure that stray placeholders get converted into tabstops
Structured snippets precisely track which text edits need to be marked
as snippet text edits, but the cases where structured snippets aren't
used but snippets are still present are for simple single text-edit
changes, so it's perfectly fine to mark all one of them as being a
snippet text edit
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 12, 2023
@DropDemBits DropDemBits marked this pull request as draft July 12, 2023 15:45
@DropDemBits
Copy link
Contributor Author

Marking as draft since I think I've found a way to test the text edit and snippet edit merging code.

Had a missing ':' between the snippet index and placeholder text
@DropDemBits DropDemBits marked this pull request as ready for review July 12, 2023 21:23
@Veykril
Copy link
Member

Veykril commented Aug 1, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 1, 2023

📌 Commit 614987a has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Aug 1, 2023

⌛ Testing commit 614987a with merge c71e136...

@bors
Copy link
Collaborator

bors commented Aug 1, 2023

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

@bors bors merged commit c71e136 into rust-lang:master Aug 1, 2023
10 checks passed
@DropDemBits DropDemBits deleted the structured-snippets-deferred-rendering branch August 1, 2023 11:51
bors added a commit that referenced this pull request Nov 12, 2023
… r=Veykril

minor: Allow multiple snippet edits in a `TextDocumentEdit`

Explicitly[^1] allow a single `TextDocumentEdit` to have multiple `SnippetTextEdit`s. This allows things like renaming extracted variables and functions without having to go through a separate rename step. For an example of what this looks like, see the video in [this comment](microsoft/vscode#145374 (comment)).

The behavior described here lines up with [what VSCode does](https://github.com/microsoft/vscode/blob/bdc113ffe148a92d0e1a8ec34b12c44ea0b73f29/src/vscode-dts/vscode.d.ts#L3728-L3731), and presumably what the eventual LSP behavior will be.

[^1]: This was technically the case before #15269, a single `TextDocumentEdit` always had multiple edits which were `InsertTextFormat.Snippet` as all of the edits were marked as being snippets, even if there weren't any tab stops or placeholders.
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.

Use InsertTextFormat.Snippet only for edits with placeholders
4 participants