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: add fallback_kinds to lsp_code_actions #2548

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fetchTe
Copy link

@fetchTe fetchTe commented Nov 6, 2024

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:

lsp-git-pr.mp4

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.

An option to specify a fallback_kinds if no only_kinds/default match
@jwortmann
Copy link
Member

jwortmann commented Nov 9, 2024

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 microsoft/language-server-protocol#1554 (comment)). 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
Copy link
Member

jwortmann commented Nov 9, 2024

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.

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