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

Filter out code actions if unsupported by the client and advertise our capabilities #4167

Merged
merged 4 commits into from
May 1, 2020

Conversation

kjeremy
Copy link
Contributor

@kjeremy kjeremy commented Apr 26, 2020

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 "refactor" so that they show up in the menu in vscode."".

Part of #144
#4147
#2833

code_action_provider: Some(CodeActionProviderCapability::Simple(true)),
code_action_provider: Some(CodeActionProviderCapability::Options(CodeActionOptions {
code_action_kinds: Some(vec![
"".to_string(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this probably needs comment (and could be String::new())

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will make a PR to lsp-types to put in Empty

// If the client only supports commands then filter the list
// and remove and actions that depend on edits.
if !world.config.client_caps.code_action_literals {
res = res
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think .retain would be a more convenietn API to use here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah no, we do need to transform things... https://doc.rust-lang.org/stable/std/vec/struct.Vec.html#method.drain_filter would work, but it's nighly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a comment to move to drain_filter once stable.

Ok(CodeAction {
title,
kind,
kind: Some("refactor".to_owned()),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems like using Empty by default, unless the kind is set explicitelly, is better?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defaulting to Empty makes sense. I just wanted things to show up in the Refactor menu but was too lazy to go through the work of restructuring everything.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I now default to Some(String::new) which I think is fine until we start assigning kinds.

Even thought we don't return all of these we eventually will so might as
well advertise now.
Most of them area. We will separate them out later but this gets them to
show up in the "refactor" menu of vscode.
@matklad
Copy link
Member

matklad commented May 1, 2020

bors r+

@bors
Copy link
Contributor

bors bot commented May 1, 2020

@bors bors bot merged commit 26079c7 into rust-lang:master May 1, 2020
@kjeremy kjeremy deleted the code-action branch May 1, 2020 19:07
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