-
Notifications
You must be signed in to change notification settings - Fork 196
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
POC for "rename" functionality #676
base: master
Are you sure you want to change the base?
Conversation
LSPs can offer a rename capability to rename a "symbol" across a workspace. We're not there yet, but this is a starting point. In VS Code it can be used by right clicking a symbol (currently variable or call to local function) and selecting "Rename symbol". A dialog should pop up asking for a new name. Official spec: https://microsoft.github.io/language-server-protocol/specifications/specification-current/#textDocument_rename
Looks promising @Tuxified. I've a few small comments and I hope they'll help you with making it complete.
I tried porting elixir 1.13 goodness from
It's stems from a limitation of EIP48 document format - it only keeps the first position. Parse the file with elixir sense. Metadata will store all found defs
Not only that. Positions in elixir and elixir_sense are in UTF8 code points, LSP needs UTF16 |
- Make rename_example a `.ex` file so we can later test multi-file refactors - Simplify rename_example to make it easier to follow - Extract some common patterns out in the tests
@lukaszsamson @Tuxified I've started a branch off of this PR to resolve those outstanding issues. Rename functionality has been updated to rename all function headers and across multiple files. I still need to fix the In the meantime I ran into some trouble when trying out the updated language server with vim.
The rename does work with other language servers so I don't think it's an issue with CoC. Just wanted to check if you had any pointers. I'll work on debugging that anyway. In terms of version support, supporting only on 1.13+ seems like a reasonable first step --> better on the 2 latest versions than not at all? WDYT?
Are there implications for this PR in terms of the different encoding of code points? |
Fix renaming functions with multiple headers and across multiple files
Cross-posting here - some more fixes for this. I've PR'd to the fork to add improvements to this branch. |
How can I help get this over the line? 😄 |
Right on, thanks @timgent. Congrats on the new addition!! 👶 🎂 You've gone beast mode here already so more than happy to chip in. |
Rebased with |
Nice one @rupurt , looking forward to seeing how you get on. Let me know if you need a hand understanding any of the stuff that is there so far |
LSPs can offer a rename capability to rename a "symbol" across a workspace.
We're not there yet, but this is a starting point. In VS Code it can be used by
right clicking a symbol (currently variable or call to local function) and
selecting "Rename symbol". A dialog should pop up asking for a new name.
Official spec:
https://microsoft.github.io/language-server-protocol/specifications/specification-current/#textDocument_rename
Please consider this as a starting point for a discussion on how to work towards (fully) implementing this functionality (this is my first PR for this project, so I'm not sure how you usually do this ;) )
This implementation is based on:
I'm unfamiliar with ElixirSense & ElixirLS, so some things are clumsy and are up for discussion:
A small preview:
Looking forward to your thoughts and I wish you a nice weekend in advance. Thanks for all your hard work, it was really helpful to be able to rely on ElixirSense for most of this :D 🤗
[edit]
This PR addresses the rename part as requested in #436