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

Add support for SnippetString in TextEdit and WorkspaceEdit #145374

Closed
jrieken opened this issue Mar 18, 2022 · 11 comments
Closed

Add support for SnippetString in TextEdit and WorkspaceEdit #145374

jrieken opened this issue Mar 18, 2022 · 11 comments
Assignees
Labels
api-finalization feature-request Request for new features or functionality on-release-notes Issue/pull request mentioned in release notes on-testplan snippets workspace-edit

Comments

@jrieken
Copy link
Member

jrieken commented Mar 18, 2022

The TextEdit and WorkspaceEdit APIs are used to apply refactorings. Those will benefit from being able to place the cursor and add placeholders, like add a placeholder for the suggested name of an extracted vairable

@jrieken jrieken self-assigned this Mar 18, 2022
@jrieken jrieken added feature-request Request for new features or functionality snippets workspace-edit labels Mar 18, 2022
@jrieken jrieken modified the milestones: Backlog, April 2022 Mar 18, 2022
@jrieken jrieken modified the milestones: April 2022, May 2022 Apr 7, 2022
@jrieken
Copy link
Member Author

jrieken commented Apr 7, 2022

For April I have enabled this (for editor dnd) and for May we can make this more prominent. Questions

  • how to handle N snippet edits
  • where to allow snippets edits
  • how do snippet edits fit into workspace edits

@jrieken
Copy link
Member Author

jrieken commented Jul 7, 2022

The actual refactoring is staged but it allow for cool things like such an enhanced extract const flow

Screen.Recording.2022-07-07.at.11.58.17.mov

jrieken added a commit that referenced this issue Jul 7, 2022
jrieken added a commit that referenced this issue Jul 7, 2022
* enroll more places into `snippetWorkspaceEdit` proposal,

#145374

* tweak API proposal for snippet edits, make this `WorkspaceEdit` only, remove old proposal bit

#145374
@jrieken jrieken modified the milestones: July 2022, August 2022 Jul 20, 2022
@zardoy
Copy link
Contributor

zardoy commented Aug 12, 2022

Hey! Thank you so much for planning this feature! I'd use it in my extension to replace some typed text into snippet.
Currently, in order to achieve that I need to a hack with onDidChangeTextDocument to track deletion before inserting snippet.

I checked it in insiders and IMO the current implementation is just ideal as it works with multiple different snippets!
Is there a plan to finally release in it August update?

@jrieken
Copy link
Member Author

jrieken commented Aug 24, 2022

Plan to finalize this API for September

@jrieken
Copy link
Member Author

jrieken commented Sep 13, 2022

This is now done via SnippetTextEdit and WorkspaceEdit.set(Uri, SnippetTextEdit[])

@jrieken
Copy link
Member Author

jrieken commented Sep 13, 2022

fyi @matklad

@zardoy
Copy link
Contributor

zardoy commented Sep 13, 2022

@jrieken as I understand, CompletionItem.additionalTextEdits won't receive snippets support, right?

Inserting/replacing content with snippets would require additional command execution for these cases. I can imagine cases, where it could be handly to replace a few lines with some snippet on completion accepted.

@jrieken
Copy link
Member Author

jrieken commented Sep 13, 2022

@jrieken as I understand, CompletionItem.additionalTextEdits won't receive snippets support, right?

As for this issue no and it needs thinking if it can be done, esp since they can come late and since the main edit can already be a snippet

@zardoy
Copy link
Contributor

zardoy commented Sep 22, 2022

@jrieken I also want to ask you about white space normalization. It'd much easier to describe the issue with the following code:

const insertString = '1\n\t1\n1'
const snippetString = new vscode.SnippetString(insertString)
const editor = vscode.window.activeTextEditor!
const insertPos = editor.selection.active
// 1
const edit = new vscode.WorkspaceEdit()
edit.set(editor.document.uri, [vscode.SnippetTextEdit.insert(insertPos, snippetString)])
void vscode.workspace.applyEdit(edit)
// 2
void editor.insertSnippet(snippetString, insertPos)

For me, these approaches to insert snippet seemed equal, but they actually insert different text (vscode.SnippetTextEdit.insert doesn't do space adjusting like editor.insertSnippet).

Is that also by design?

@jrieken
Copy link
Member Author

jrieken commented Sep 26, 2022

Is that also by design?

Fair question and something I didn't consider yet. Don't know yet what the design should be but looking at the sources they should behave the same: both should default to adjusting whitespace. I will investigate

@zardoy
Copy link
Contributor

zardoy commented Sep 26, 2022

Fair question and something I didn't consider yet. I will investigate

If I lived in ideal world I'd see an option to also disable this behavior :) IMO these two cases I described above should be aligned with what accepting completion does. e.g. ok if it does space adjusting by default, but there also should be away to disable that, like we can disable it with keepWhitespace=false for suggestions. Hope you will consider that 🙏

I will investigate

Thank you so much, I really appreciate the work you're doing here! Let me know how it goes!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-finalization feature-request Request for new features or functionality on-release-notes Issue/pull request mentioned in release notes on-testplan snippets workspace-edit
Projects
None yet
Development

No branches or pull requests

3 participants