Categorize assists #5116
Categorize assists #5116
Conversation
kjeremy
force-pushed
the
code-action-kind
branch
from
2c4207f to
4dbf56a
Compare
crates/ra_assists/src/lib.rs
Outdated
| RefactorRewrite, | ||
| Source, | ||
| OrganizeImports, | ||
| } |
There was a problem hiding this comment.
Let' keep only those kinds which we actually use.
| @@ -52,7 +52,7 @@ pub(crate) fn add_custom_impl(acc: &mut Assists, ctx: &AssistContext) -> Option< | |||
| format!("Add custom impl `{}` for `{}`", trait_token.text().as_str(), annotated_name); | |||
|
|
|||
| let target = attr.syntax().text_range(); | |||
| acc.add(AssistId("add_custom_impl"), label, target, |builder| { | |||
| acc.add(AssistId("add_custom_impl"), AssistKind::Refactor, label, target, |builder| { | |||
There was a problem hiding this comment.
Wdyt, does it make sense to move AssistKind into AssistId struct? Kind is kind-of determined by ID.
There was a problem hiding this comment.
On the other hand moving it into the AssistId struct makes it seem like AssistId("add_custom_impl", AssistKind::Refactor) and AssistId("add_custom_impl", AssistKind::RefactorExtract) are two separate assists.
| }) | ||
| acc.add( | ||
| AssistId("add_function"), | ||
| AssistKind::RefactorExtract, |
There was a problem hiding this comment.
This is not extract though, but rather create from use. Ie, it doesn't wrap existing code into a function
| .and_then(|impl_def| { | ||
| acc.add( | ||
| AssistId("add_new"), | ||
| AssistKind::Refactor, |
There was a problem hiding this comment.
Let's add AssistKind::Generate for things which generate new code (add constructor, add impl)
There was a problem hiding this comment.
I'm now thinking that there's no reason to do this and we should just leave this kind as empty since there's no UI need to distinguish this case.
There was a problem hiding this comment.
Heh, my reason for suggesting this was that IntelliJ actually has a dedicated UI for generating stuff: https://www.jetbrains.com/help/idea/generating-code.html :-)
There was a problem hiding this comment.
Definitely. I think if we get a menu to do that then we should re-categorize.
| }) | ||
| acc.add( | ||
| AssistId("fill_match_arms"), | ||
| AssistKind::RefactorRewrite, |
| }; | ||
| acc.add( | ||
| AssistId("move_guard_to_arm_body"), | ||
| AssistKind::RefactorExtract, |
| let ok_tuple = make::tuple_struct_pat(ok_path, iter::once(it)).into(); | ||
| acc.add( | ||
| AssistId("replace_unwrap_with_match"), | ||
| AssistKind::RefactorRewrite, |
There was a problem hiding this comment.
Hm, let's introduce two kinds for:
- assist that rewrite code to equivalent (apply de-morgan, replace if-let with match, etc)
- assists that may change semantics of code
Not sure what are the best names for two things...
There was a problem hiding this comment.
According to the lsp spec:
The set of kinds is open and client needs to announce the kinds it supports to the server during initialization.
This means that at least if you introduce new kinds, you would have to support a fallback when the client doesn't say it supports them.
There was a problem hiding this comment.
This means that at least if you introduce new kinds, you would have to support a fallback when the client doesn't say it supports them.
It's the client job to handle unknown values:
- The code action kind values the client supports. When this
- property exists the client also guarantees that it will
- handle values outside its set gracefully and falls back
- to a default value when unknown.
| @@ -28,7 +28,7 @@ pub(crate) fn split_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> | |||
| } | |||
|
|
|||
| let target = colon_colon.text_range(); | |||
| acc.add(AssistId("split_import"), "Split import", target, |edit| { | |||
| acc.add(AssistId("split_import"), AssistKind::RefactorExtract, "Split import", target, |edit| { | |||
|
We should also think about filtering: the LSP request can specify that it's only interested in certain assists and our infra makes it hard to avoid descending into the handlers based on this information. |
|
I think this is ready for another round of review. |
Categorize assists so that editors can use them. Follows the LSP spec pretty close (and some things may need adjustments) but this populates the Refactor menu in vscode and pushes quickfixes through again.
This is a prerequisite to filtering out assists that the client doesn't care about.
Fixes #4147