antosha417 / nvim-lsp-file-operations

Neovim plugin that adds support for file operations using built-in LSP
Apache License 2.0
301 stars 17 forks source link

New integration from yazi.nvim 👋🏻 #35

Open mikavilpas opened 4 months ago

mikavilpas commented 4 months ago

Hi! I am working with a neovim plugin for a terminal file manager. I added support for renaming and deleting files using your implementation (love it).

I was wondering if you could skim over my implementation here https://github.com/mikavilpas/yazi.nvim/pull/190

Also, I asked for your plugin to be packaged into luarocks here https://github.com/nvim-neorocks/nurr/pull/14 so that I can depend on it from my continuous integration test suite.

antosha417 commented 3 months ago

Hi! Thanks for reaching out and for using the plugin.

I've looked through your pr briefly, One concern that I have is calling both will-delete and did-delete at the same time. I haven't seen any language servers that implement both will-delete and did-delete yet. But I have a feeling that if there are both of these methods only one of them should be called.

will-delete should be called before actually moving the files. You should wait for the workspace edit from language server, apply it, and then move the files.

did-delete should be called after moving the files. You should then apply workspace edit to fix all the imports.

Same applies for renaming files.

Regarding initialization logic, you can set config values that you need. If there is something missing I'm always open for new prs. If the current event-based integration meets your needs without any issues you are good to go.

As for importing internal modules like lsp-file-operations.did-rename, I don't expect them to change much.

Thanks for packaging the plugin into Luarocks!

Feel free to reach out if you have any more questions or need further assistance.

mikavilpas commented 3 months ago

Thanks for the feedback. I'll try to see if I can avoid issues with calling both will-rename and did-rename as it does seem a bit weird to me too.

The use case I have with yazi (the terminal file manager) is currently not fully compliant with the LSP specification, I think. This is because the renaming is done by yazi before I can send that will-rename event.

Currently I don't have a way to improve it, but I might have a way in the future. Right now my idea is to have a "50% solution" that will help in some cases and just accept that some LSPs cannot be supported.

One thing I could ask for help for though is the possibility for adding versioned releases to nvim-lsp-file-operations. It was suggested as a potential solution to me in https://github.com/nvim-neorocks/nvim-busted-action/issues/4#issuecomment-2227314452. Right now the nvim-lsp-file-operations luarocks package is published as an scm version, which I think means it's a development version.

There is an ongoing issue where the unit testing github action that I depend on doesn't seem to be able to fetch development versions of luarocks. In my plugin I am using https://github.com/marketplace/actions/release-please-action, but my guess is the logic for determining the version number looks for git tags in the form of v1.0.0 (example run)