Skip to content

Assign CodeAction Kind #2833

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

Closed
kjeremy opened this issue Jan 13, 2020 · 9 comments
Closed

Assign CodeAction Kind #2833

kjeremy opened this issue Jan 13, 2020 · 9 comments

Comments

@kjeremy
Copy link
Contributor

kjeremy commented Jan 13, 2020

In order to nicely support multiple code actions at a given location we need to fill in the kind field for each action in a hierarchical fashion.

See https://microsoft.github.io/language-server-protocol/specifications/specification-3-15/#textDocument_codeAction and handle_code_action in handlers.rs

See https://code.visualstudio.com/updates/v1_41#_codeactiondisabled which suggests we can append to the builtin strings?

@matklad
Copy link
Member

matklad commented Jan 13, 2020

cc @SomeoneToIgnore, this seems(I haven’t read the spec yet) like it might actually cover the import use-case as well?

@SomeoneToIgnore
Copy link
Contributor

SomeoneToIgnore commented Jan 14, 2020

I will try it again, but I was unable to get those actions to group by setting that property and enabling the support on the client and the server:

#2716 (comment)

@kjeremy
Copy link
Contributor Author

kjeremy commented Jan 14, 2020

@SomeoneToIgnore did you change the titles to unique things too? I would expect that to work but... the LSP isn't exactly obvious sometimes.

@kjeremy
Copy link
Contributor Author

kjeremy commented Jan 14, 2020

At the very least this will give us keybinding support https://code.visualstudio.com/docs/editor/refactoring

@SomeoneToIgnore
Copy link
Contributor

SomeoneToIgnore commented Jan 16, 2020

image

Here's what I get with different names and different kinds.
Here's the hacky code I used to get this: https://github.com/rust-analyzer/rust-analyzer/compare/master...SomeoneToIgnore:multiple-actions-test?expand=1#diff-57a6a05c79db4a0461f95762e92206d3

If feels like changing supported actions and code action kinds does nothing, that's why I wonder what I miss.

@kjeremy
Copy link
Contributor Author

kjeremy commented Jan 16, 2020

In VSCode if you hit the keybinding for refactoring (ctrl+shift+R) the UI should only return refactors. "ctrl+." should give you both refactors and quickfixes. We can actually use this information to ignore computing code assists on the server side too I think. There is also a kind that can apply all quick fixes to the file (source.fixAll).

If you assign is_preferred and then assign a keybinding you can autofix if an action is marked as a quickfix. It looks like there is currently no grouping at the menu level but I'm pretty sure I saw an issue about it (I'll dig it up).

@kjeremy
Copy link
Contributor Author

kjeremy commented Jan 16, 2020

To summarize I think we should do the following:

  1. Make code action titles (and associates command titles?) dynamic if possible based on what they will do.
  2. Ensure that we can return multiple instances of the same AssistId kind for a given location.
  3. Assign the appropriate CodeActionKind to all assists (to start this could just be the default list from the LSP protocol, we don't have to get hierarchical until we think it will change behavior on the client side).
  4. Possibly filter the assist computations based on the set passed in the CodeActionContext of the params.
  5. Figure out what makes sense for marking an action as preferred.

For filtering (and eventually marking assists as 'non applicable' ie disabled) we should probably know our AssistId list upfront. For figuring out the CodeActionKind we could possibly start with an enum on AssistId and throw a conversion in conv.rs @matklad?

@kjeremy
Copy link
Contributor Author

kjeremy commented Jan 16, 2020

@kjeremy
Copy link
Contributor Author

kjeremy commented Apr 24, 2020

I was just playing with this and if we return the kinds from the server initialize request we get the refactor and code action menus showing up in vscode.

Filling in the kind field also allows them to show up in the menus as those actions.

bors bot added a commit that referenced this issue May 1, 2020
4167: Filter out code actions if unsupported by the client and advertise our capabilities r=matklad a=kjeremy

This PR does three things:
1. If the client does not support `CodeActionKind` this will filter the results and only send `Command[]` back.
2. Correctly advertises to the client that the server supports `CodeActionKind`. This may cause clients to not request code actions if they are checking for the provider to be `true` (or implement LSP < 3.8) in the caps but I will fix that in a followup PR.
3. Marks most CodeActions as <strike>"refactor" so that they show up in the menu in vscode.</strike>`""`.

Part of #144
#4147 
#2833  

Co-authored-by: kjeremy <[email protected]>
@kjeremy kjeremy closed this as completed Jul 13, 2020
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

No branches or pull requests

3 participants