elixir-lsp
POC for "rename" functionality
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:
- Elixir 1.13s Code.Fragment.surround_context/2 (to understand what kind of symbol/token we're dealing with)
- ElixirSense.definition & ElixirSense.references (to figure out at which locations we need changes)
I'm unfamiliar with ElixirSense & ElixirLS, so some things are clumsy and are up for discussion:
- ElixirSense.definition returns the first definition (so a function with multiple heads don't get fully renamed)
- ElixirSense.definition & references return %Location{} without file (so POC only works for rename across current file)
- LSP communicates with 0-based locations (line & column) but ElixirSense, Code.Fragment etc use 1-based locations so the translation look clumsy ( +1 & -1 on wrong places?)
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 :hugs:
[edit]
This PR addresses the rename part as requested in #436
Looks promising @Tuxified. I've a few small comments and I hope they'll help you with making it complete.
Elixir 1.13s Code.Fragment.surround_context/2 (to understand what kind of symbol/token we're dealing with)
I tried porting elixir 1.13 goodness from Code.Fragment to earlier elixir versions (I did that for 1.12 stuff) but without 1.13 tokenizer those features cannot work. We need to either do some kind of a backport (see e.g. ElixirSense.Source.subject) or decide we only support this on 1.13+
ElixirSense.definition & references return %Location{} without file (so POC only works for rename across current file)
nil file means the current text buffer. When it returns a binary, it's a path.
ElixirSense.definition returns the first definition (so a function with multiple heads don't get fully renamed)
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
LSP communicates with 0-based locations (line & column) but ElixirSense, Code.Fragment etc use 1-based locations so the translation look clumsy ( +1 & -1 on wrong places?)
Not only that. Positions in elixir and elixir_sense are in UTF8 code points, LSP needs UTF16
@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 prepare functionality across files though. That'll probably not be for another couple of weeks when I get time.
In the meantime I ran into some trouble when trying out the updated language server with vim.
[coc.nvim] Error on applyEdits: Error: file:///.... changed before apply edit
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?
Not only that. Positions in elixir and elixir_sense are in UTF8 code points, LSP needs UTF16
Are there implications for this PR in terms of the different encoding of code points?
Cross-posting here - some more fixes for this. I've PR'd to the fork to add improvements to this branch.
https://github.com/Tuxified/elixir-ls/pull/2
@rupurt checkout the latest comments on this version of the PR - https://github.com/elixir-lsp/elixir-ls/pull/791 - which is slightly further along.
I've recently had a kid so not going to have time to work on it in the foreseeable
Right on, thanks @timgent. Congrats on the new addition!! :baby: :birthday:
You've gone beast mode here already so more than happy to chip in.
Rebased with master and started trying to comprehend the work you've done and how it needs to be updated to handle the new app structure along with ElixirSense v2. Thank you for creating the examples and failing tests @timgent. Gives me a great starting point.
https://github.com/elixir-lsp/elixir-ls/compare/master...rupurt:elixir-ls:rupurt/tuxified/poc_rename_symbol?expand=1
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