LSP: Support textDocument/prepareRename #6103
Merged
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
'textDocument/prepareRename' can be used by the client to ask the server the range of the symbol under the cursor which would be changed by a subsequent call to 'textDocument/rename' with that position. We can use this information to fill the prompt with an accurate prefill which can improve the UX for renaming symbols when the symbol doesn't align with the "word" textobject. (We currently use the "word" textobject as a default value for the prompt.) Co-authored-by: Michael Davis <[email protected]>
the-mikedavis
added
C-enhancement
Comment on lines
+1323
to
+1324
| Err(e) => { | ||
| editor.set_error(e); |
There was a problem hiding this comment.
If we return early here we won't be able to try the rename. That might be ok: if get_prefill_from_lsp_response returns an Err, the language server either doesn't support renaming at the current position or it sent an invalid range which is a problem too.
Alternatively though we could fall back to the word textobject prefill value here. I'm not sure which is better behavior. Let's leave it as-is for now?
There was a problem hiding this comment.
I don't have the time to investigate myself right now but it might be worth looking at what vscode does here
There was a problem hiding this comment.
I couldn't find exactly where this is handled in vscode but it looks like the current behavior here matches nvim's https://github.com/neovim/neovim/blob/ca3bc56a3b1b3454498a4a23c0d700486d554077/runtime/lua/vim/lsp/buf.lua#L319-L379
Co-authored-by: Michael Davis <[email protected]>
Co-authored-by: Michael Davis <[email protected]>
This was
linked to
issues
Mar 5, 2023
Co-authored-by: Michael Davis <[email protected]>
the-mikedavis
approved these changes
Mar 6, 2023
archseer
approved these changes
Mar 8, 2023
sagnibak
pushed a commit
to sagnibak/helix
that referenced
this pull request
Mar 21, 2023
* LSP: Support textDocument/prepareRename 'textDocument/prepareRename' can be used by the client to ask the server the range of the symbol under the cursor which would be changed by a subsequent call to 'textDocument/rename' with that position. We can use this information to fill the prompt with an accurate prefill which can improve the UX for renaming symbols when the symbol doesn't align with the "word" textobject. (We currently use the "word" textobject as a default value for the prompt.) Co-authored-by: Michael Davis <[email protected]> * clippy fixes * rustfmt * Update helix-term/src/commands/lsp.rs Co-authored-by: Michael Davis <[email protected]> * Update helix-term/src/commands/lsp.rs Co-authored-by: Michael Davis <[email protected]> * fix clippy from suggestions * Update helix-term/src/commands/lsp.rs Co-authored-by: Michael Davis <[email protected]> --------- Co-authored-by: Michael Davis <[email protected]>
wes-adams
pushed a commit
to wes-adams/helix
that referenced
this pull request
Jul 4, 2023
* LSP: Support textDocument/prepareRename 'textDocument/prepareRename' can be used by the client to ask the server the range of the symbol under the cursor which would be changed by a subsequent call to 'textDocument/rename' with that position. We can use this information to fill the prompt with an accurate prefill which can improve the UX for renaming symbols when the symbol doesn't align with the "word" textobject. (We currently use the "word" textobject as a default value for the prompt.) Co-authored-by: Michael Davis <[email protected]> * clippy fixes * rustfmt * Update helix-term/src/commands/lsp.rs Co-authored-by: Michael Davis <[email protected]> * Update helix-term/src/commands/lsp.rs Co-authored-by: Michael Davis <[email protected]> * fix clippy from suggestions * Update helix-term/src/commands/lsp.rs Co-authored-by: Michael Davis <[email protected]> --------- Co-authored-by: Michael Davis <[email protected]>
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.
@the-mikedavis here's the updated branch with fallback behavior in all the right places, I think. I used inner functions to factor our duplicate logic and actually think it came out quite nice. Seen it elsewhere in the codebase but curious if it's considered a "good" pattern we want to embrace.
'textDocument/prepareRename' can be used by the client to ask the server the range of the symbol under the cursor which would be changed by a subsequent call to 'textDocument/rename' with that position.
We can use this information to fill the prompt with an accurate prefill which can improve the UX for renaming symbols when the symbol doesn't align with the "word" textobject. (We currently use the "word" textobject as a default value for the prompt.)