Skip to content

Symbol Rename#131

Open
Mandarancio wants to merge 8 commits into
lite-xl:masterfrom
Mandarancio:master
Open

Symbol Rename#131
Mandarancio wants to merge 8 commits into
lite-xl:masterfrom
Mandarancio:master

Conversation

@Mandarancio

Copy link
Copy Markdown

Implementation of symbol renaming using the results from the lsp server:

  • using get_token_range when no selection is active to get the symbol under the cursor
  • using the result from lsp to rename the symbol across the project:
    • open all affected document
    • select and replace all symbol occurencies

I 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

@Mandarancio Mandarancio marked this pull request as draft July 10, 2024 16:23
@jgmdev

jgmdev commented Jul 10, 2024

Copy link
Copy Markdown
Member

I have implemented this some time ago with support for previewing and approving the changes graphically pragtical/lsp#3

@Mandarancio

Copy link
Copy Markdown
Author

@jgmdev nice, there are any plans to merge it into lite-xl-lsp?

@jgmdev

jgmdev commented Jul 10, 2024

Copy link
Copy Markdown
Member

Can be picked by any one for merging.

@Guldoman Guldoman left a comment

Copy link
Copy Markdown
Member

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).

Comment thread init.lua Outdated
Comment thread init.lua Outdated
Comment thread init.lua Outdated
@Guldoman Guldoman linked an issue Jul 12, 2024 that may be closed by this pull request
@Mandarancio

Mandarancio commented Jul 12, 2024

Copy link
Copy Markdown
Author

I just realized that different LSP have different responses, for example clangd will reply with something like this

{
  "results": {
    "changes": {
      "$FILE_URI_0": [ ... ],
      "$FILE_URI_1": [ ... ]
    }
  }
}

While pyright will answer like:

{
  "results": {
    "documentChanges": [
      {
        "textDocument": { "uri": "$FILE_URI_0" },
        "edits": [ ... ]
      }, {
        "textDocument": { "uri": "$FILE_URI_1" },
        "edits": [ ... ]
      }
    ]
  }
}

There is a way to uniform the responses?

@Mandarancio

Copy link
Copy Markdown
Author

I just realized that different LSP have different responses, for example clangd will reply with something like this

{
  "results": {
    "changes": {
      "$FILE_URI_0": [ ... ],
      "$FILE_URI_1": [ ... ]
    }
  }
}

While pyright will answer like:

{
  "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

@Guldoman Guldoman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more things:

  1. Set our client capabilities (in server.lua) to support textDocument.rename (but not the "prepared" part).
  2. Check the server capability (renameProvider) before sending the request.
  3. If the server capability is set, add a rename entry in the context menu.

Comment thread init.lua
for file_uri, changes in pairs(response.result.changes) do
core.log(file_uri .. " " .. #changes)
-- TODO: Finish implement textDocument/rename
if response.result then

Copy link
Copy Markdown
Member

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.

@Mandarancio Mandarancio Jul 18, 2024

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Done
  2. It was already checked
  3. I have doubt how I can access to the document servers at the time of the context menu initialisation
  4. Done

Comment thread init.lua Outdated
Comment thread init.lua Outdated
@Mandarancio Mandarancio marked this pull request as ready for review July 22, 2024 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rename Symbol not working

3 participants