Skip to content
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

feat: Shim vim.ui.select to reuse as much of the code_action code #48

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

IndianBoy42
Copy link
Contributor

@IndianBoy42 IndianBoy42 commented May 14, 2024

In-progress PR because I noticed that a large part of this repo is a copy of
vim.lsp.buf.code_action, which has changed since it was copied into this
repository. To make it less possible for it to get out of date we can just call
vim.lsp.buf.code_action directly and use a shim that intercepts
vim.ui.select to use the actions-preview backend only if kind == "codeaction".

I chose to add the shim the first time actions-preview is called, otherwise
(say if it was wrapped in setup) if actions-preview is loaded before
something else replaces vim.ui.select then we lose the shim. There may be
some edge-cases around this wrapping that needs to be considered.

TODO:

  • does code action resolve work properly?
  • test and fix the nui backend

TODO: does code action resolve work properly?
@IndianBoy42
Copy link
Contributor Author

Please help testing, I haven't found any problems with rust-analyzer using it for a few days, if you use another LSP then it would be good to check if it does something different.

@aznhe21
Copy link
Owner

aznhe21 commented Jun 10, 2024

Great job! Since breaking changes are acceptable, it would be good to make require(“actions-preview”).setup() mandatory. It would also be good to make require(“actions-preview”).code_actions() a function that only issues warnings as backward compatibility.

I will test this for a several days.

@aznhe21
Copy link
Owner

aznhe21 commented Jun 10, 2024

Ah, but with this change, we can't enforce the use of actions-preview.nvim, can we? It might cause issues when used alongside dressing.nvim, etc. It would be great if there's a solution for this.

@IndianBoy42
Copy link
Contributor Author

So currently the way it works is that you still have to call actions-preview.code_action and there I override vim.ui.select with our handler. Our handler passes invocations with the wrong kind argument to the original handler. So our handler works if its installed after other plugins that override the vim.ui.select. If i only installed the handler in setup then I think other plugins might override (probably without passthrough), so AFAICT we have to have actions-preview.code_action().

I wish there was a more fundamental direct way to do this kind of "override for only a specific kind" thing. There was some discussion in github issues about a provider based method but I don't know if its backwards compatible so it might not happen. Currently I check if vim.ui.select is assigned to our function and don't reoverride. But if another plugin tried to do what we did then we'd keep stacking handlers on top of each other.

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.

2 participants