-
Notifications
You must be signed in to change notification settings - Fork 21
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
Symbol Rename #131
base: master
Are you sure you want to change the base?
Symbol Rename #131
Conversation
I have implemented this some time ago with support for previewing and approving the changes graphically pragtical/lsp#3 |
@jgmdev nice, there are any plans to merge it into lite-xl-lsp? |
Can be picked by any one for merging. |
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.
For now this approach is probably fine, and allows to relatively easily revert the changes.
In the future, if we want to add some interface for it, we'll design something (for example something like what I do in the copilot plugin for the "panel completions", but a bit more complete).
I just realized that different LSP have different responses, for example {
"results": {
"changes": {
"$FILE_URI_0": [ ... ],
"$FILE_URI_1": [ ... ]
}
}
} While {
"results": {
"documentChanges": [
{
"textDocument": { "uri": "$FILE_URI_0" },
"edits": [ ... ]
}, {
"textDocument": { "uri": "$FILE_URI_1" },
"edits": [ ... ]
}
]
}
} There is a way to uniform the responses? |
Implemented workaround that convert the two types of answer to a same structure |
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.
A few more things:
- Set our client capabilities (in
server.lua
) to supporttextDocument.rename
(but not the "prepared" part). - Check the server capability (
renameProvider
) before sending the request. - If the server capability is set, add a rename entry in the context menu.
for file_uri, changes in pairs(response.result.changes) do | ||
core.log(file_uri .. " " .. #changes) | ||
-- TODO: Finish implement textDocument/rename | ||
if response.result then |
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.
Also check for response.error
and log it out.
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.
- Done
- It was already checked
- I have doubt how I can access to the document servers at the time of the context menu initialisation
- Done
Implementation of symbol renaming using the results from the lsp server:
get_token_range
when no selection is active to get the symbol under the cursorI did not implement an automatic file save of the changes as it force the developer to open, (review), and save the changes manually (it could be a configuration).
There is still work to do as the LSP after the refactoring seems to be out of sync with the updated files, help with testing and debugging is needed