-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Support snippet text edit #4494
Conversation
Build failed: |
bors r+ |
Build failed: |
bors r+ |
Build failed: |
bors r+ |
Build failed: |
// We intentionally don't support command-based actions, as those either | ||
// requires custom client-code anyway, or requires server-initiated edits. | ||
// Server initiated edits break causality, so we avoid those as well. | ||
if !world.config.client_caps.code_action_literals { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @kjeremy
I think, given that pure commands require us to implement client-specific logic anyway, I figured that we might as well just not support them? This is related to the caps PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's only true until we support workspace/executeCommand
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's only true until we support
workspace/executeCommand
.
I personally think that we should not support this :-) The only command that the server can execute is sending workspace edit to the client, but that's strictly worse than just sending it with edit.
Ie, I think it is confirming to the protocol and a good tradeoff to just not support clients that can do commands with edits.
bors r+ |
Canceled. |
bors r+ |
Build succeeded: |
``` | ||
|
||
When applying such code action, the editor should insert snippet, with tab stops and placeholder. | ||
At the moment, rust-analyzer guarantees that only a single edit will have `InsertTextFormat.Snippet`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @fannheyward @brotzeit, we are going to replace rust-analyzer.applySourceChange
command with something which hopefully will make it into the procotol.
Specifically, if client does not specify experimental.snippetTextEdit
, we'll use just the stock CodeAction
s with edit
s.
If client sets this capablity though, we will be sending snippets in edit
s.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
microsoft/language-server-protocol#724 is the upstream issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
created #4604 as a more structured way to send pings :D
coc-rust-analyzer has added snippet text edit support. fannheyward/coc-rust-analyzer#252, anyone can give this a test? |
* feat(cmds): support snippet text edit rust-lang/rust-analyzer#4494 * feat(cmds): improve snippet text edit * feat(cmds): snippet textedit
* feat(cmds): support snippet text edit rust-lang/rust-analyzer#4494 * feat(cmds): improve snippet text edit * feat(cmds): snippet textedit
bors r+
🤖