LSP icon indicating copy to clipboard operation
LSP copied to clipboard

feat: add fallback_kinds to lsp_code_actions

Open fetchTe opened this issue 1 year ago • 2 comments

An option to specify a fallthrough kind via fallback_kinds if only_kinds/default does not have a code action.

This is somewhat related to #1356, as it allows a fallback mechanism within a single keybinding, similar to .sublime-keymap's handling of keybinding fallthrough. Maybe I've overlooked something and this is possible, but if not, here's a short video demo of it in action:

https://github.com/user-attachments/assets/b9de7fda-1721-467a-a564-e270a8bcddb2

The accompanying .sublime-keymap:

[{
  "keys": ["super+h"],
  "command": "lsp_code_actions",
  "args": {"fallback_kinds": ["source"]},
  "context": [{"key": "lsp.session_with_capability", "operand": "codeActionProvider.codeActionKinds"}]
}]

Typically achieved via something like this (does not work):

[{
  "keys": [ "super+h" ],
  "command": "lsp_code_actions",
  "args": {"only_kinds": ["source"]},
  "context": [
    { "key": "lsp.session_with_capability","operand": "codeActionProvider.codeActionKinds" }
  ]
}, {
  "keys": [ "super+h" ],
  "command": "lsp_code_actions",
  "context": [
    { "key": "lsp.session_with_capability","operand": "codeActionProvider.codeActionKinds" }
  ]
}]

That said, a simpler solution is to just use two separate key bindings, so it's a-ok if you decide not to merge this PR.

fetchTe avatar Nov 06 '24 22:11 fetchTe

Not sure if I entirely understand what you are trying to achieve.

Usually code actions are related to your cursor position; e.g. to fix a particular diagnostic, refactor something, etc. Code actions with the "source" kind on the other hand apply to the entire file. I don't know why this kind was named "source", but whatever, it shouldn't be important. You can always access the "source" actions from the main menu under Edit > LSP: Source Action or from the context menu.

Some servers like ESLint seem to not provide "source" actions unless only (named "only_kinds" in the command argument) is specified. This is a server decision though, not something which is demanded or suggested in the protocol specs. When you trigger the actions with the cursor at some code, LSP-eslint only returns the relevant actions (kind "quickfix") for that cursor position. We can see that in your video where the "source" action ("fix all issues...") is indeed not included in that case. I'm not sure what's the idea behind this decision from ESLint, but it might be something evoked by certain behavior in VSCode (more context at https://github.com/microsoft/language-server-protocol/issues/1554#issuecomment-1266512306). Due to that, some servers might even have special handling to "cheat" for the returned code action kind if it detects that the client is VSCode: https://github.com/julia-vscode/LanguageServer.jl/blob/0da181a8e48da14c2ac46519311e01352c7c6fca/src/requests/actions.jl#L58-L62

So the handling implemented in this PR is kind of a special case for ESLint, perhaps also other servers which might do the same. I assume that what you actually would like to have is that the quick panel should always show all code actions, i.e. including both "quickfix" and "source" at the same time? I'm a bit skeptical whether this "send another request if the first response is empty" is the right approach. Ideally ESLint would just include the "source" actions in the response too. Alternatively this client could do two requests from the beginning and merge the results, but that also seems to be specific to the ESLint behavior and we would need to deduplicate results if the server already includes the "source" actions in the regular request.

Btw your second key binding example has no effect because both bindings share the exact same context. That means the last one where the context matches is used and the other one is entirely ignored.

jwortmann avatar Nov 09 '24 16:11 jwortmann

~~Besides what I wrote above, I think the implementation from this PR would result in an infinite loop if fallback_kinds was given but the server doesn't return any code actions for those.~~ Ah sorry on second look I see it doesn't. But still it feels too "hacky" to me and not like a good approach.

jwortmann avatar Nov 09 '24 17:11 jwortmann

I agree that this solution is not that good. It's too specific to one use case.

Perhaps a viable alternative would be to add a context like has_diagnostics and have two key bindings - one with the context that would trigger lsp_code_actions and one without context that would trigger lsp_source_action.

It might not work exactly the same but it might work as well as this in practice (if we assume that having a diagnostic means that there is always a quick fix).

rchl avatar Oct 04 '25 13:10 rchl