From fb99831cb044e5fbf171bdd950a7486a01ce0d51 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 9 Feb 2020 14:30:27 +0100 Subject: Slightly simpler API for groups --- crates/ra_assists/src/assist_ctx.rs | 92 ++++++++++++++++++--------- crates/ra_assists/src/handlers/auto_import.rs | 33 ++++------ 2 files changed, 77 insertions(+), 48 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/assist_ctx.rs b/crates/ra_assists/src/assist_ctx.rs index 81f999090..5924a3fd5 100644 --- a/crates/ra_assists/src/assist_ctx.rs +++ b/crates/ra_assists/src/assist_ctx.rs @@ -98,30 +98,8 @@ impl<'a> AssistCtx<'a> { Some(assist) } - pub(crate) fn add_assist_group( - self, - id: AssistId, - label: impl Into, - f: impl FnOnce() -> Vec, - ) -> Option { - let label = AssistLabel::new(label.into(), id); - let assist = if self.should_compute_edit { - let actions = f(); - assert!(!actions.is_empty(), "Assist cannot have no"); - - Assist::Resolved { - assist: ResolvedAssist { - label, - action_data: Either::Right( - actions.into_iter().map(ActionBuilder::build).collect(), - ), - }, - } - } else { - Assist::Unresolved { label } - }; - - Some(assist) + pub(crate) fn add_assist_group(self, group_name: impl Into) -> AssistGroup<'a> { + AssistGroup { ctx: self, group_name: group_name.into(), assists: Vec::new() } } pub(crate) fn token_at_offset(&self) -> TokenAtOffset { @@ -155,6 +133,67 @@ impl<'a> AssistCtx<'a> { } } +pub(crate) struct AssistGroup<'a> { + ctx: AssistCtx<'a>, + group_name: String, + assists: Vec, +} + +impl<'a> AssistGroup<'a> { + pub(crate) fn add_assist( + &mut self, + id: AssistId, + label: impl Into, + f: impl FnOnce(&mut ActionBuilder), + ) { + let label = AssistLabel::new(label.into(), id); + + let assist = if self.ctx.should_compute_edit { + let action = { + let mut edit = ActionBuilder::default(); + f(&mut edit); + edit.build() + }; + Assist::Resolved { assist: ResolvedAssist { label, action_data: Either::Left(action) } } + } else { + Assist::Unresolved { label } + }; + + self.assists.push(assist) + } + + pub(crate) fn finish(self) -> Option { + assert!(!self.assists.is_empty()); + let mut label = match &self.assists[0] { + Assist::Unresolved { label } => label.clone(), + Assist::Resolved { assist } => assist.label.clone(), + }; + label.label = self.group_name; + let assist = if self.ctx.should_compute_edit { + Assist::Resolved { + assist: ResolvedAssist { + label, + action_data: Either::Right( + self.assists + .into_iter() + .map(|assist| match assist { + Assist::Resolved { + assist: + ResolvedAssist { label: _, action_data: Either::Left(it) }, + } => it, + _ => unreachable!(), + }) + .collect(), + ), + }, + } + } else { + Assist::Unresolved { label } + }; + Some(assist) + } +} + #[derive(Default)] pub(crate) struct ActionBuilder { edit: TextEditBuilder, @@ -164,11 +203,6 @@ pub(crate) struct ActionBuilder { } impl ActionBuilder { - /// Adds a custom label to the action, if it needs to be different from the assist label - pub(crate) fn label(&mut self, label: impl Into) { - self.label = Some(label.into()) - } - /// Replaces specified `range` of text with a given string. pub(crate) fn replace(&mut self, range: TextRange, replace_with: impl Into) { self.edit.replace(range, replace_with.into()) diff --git a/crates/ra_assists/src/handlers/auto_import.rs b/crates/ra_assists/src/handlers/auto_import.rs index 4514b8691..d13332f37 100644 --- a/crates/ra_assists/src/handlers/auto_import.rs +++ b/crates/ra_assists/src/handlers/auto_import.rs @@ -1,12 +1,8 @@ -use hir::ModPath; use ra_ide_db::imports_locator::ImportsLocator; -use ra_syntax::{ - ast::{self, AstNode}, - SyntaxNode, -}; +use ra_syntax::ast::{self, AstNode}; use crate::{ - assist_ctx::{ActionBuilder, Assist, AssistCtx}, + assist_ctx::{Assist, AssistCtx}, insert_use_statement, AssistId, }; use std::collections::BTreeSet; @@ -67,19 +63,18 @@ pub(crate) fn auto_import(ctx: AssistCtx) -> Option { return None; } - ctx.add_assist_group(AssistId("auto_import"), format!("Import {}", name_to_import), || { - proposed_imports - .into_iter() - .map(|import| import_to_action(import, &position, &path_to_import_syntax)) - .collect() - }) -} - -fn import_to_action(import: ModPath, position: &SyntaxNode, anchor: &SyntaxNode) -> ActionBuilder { - let mut action_builder = ActionBuilder::default(); - action_builder.label(format!("Import `{}`", &import)); - insert_use_statement(position, anchor, &import, action_builder.text_edit_builder()); - action_builder + let mut group = ctx.add_assist_group(format!("Import {}", name_to_import)); + for import in proposed_imports { + group.add_assist(AssistId("auto_import"), format!("Import `{}`", &import), |edit| { + insert_use_statement( + &position, + path_to_import_syntax, + &import, + edit.text_edit_builder(), + ); + }); + } + group.finish() } #[cfg(test)] -- cgit v1.2.3 From 9769c5140c9c406a4cc880e698593a6c4bcc6826 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 9 Feb 2020 15:32:53 +0100 Subject: Simplify Assists interface Instead of building a physical tree structure, just "tag" related assists with the same group --- crates/ra_assists/src/assist_ctx.rs | 83 +++++++++++++++++-------------------- crates/ra_assists/src/doc_tests.rs | 2 +- crates/ra_assists/src/lib.rs | 64 +++++++--------------------- 3 files changed, 54 insertions(+), 95 deletions(-) (limited to 'crates/ra_assists') diff --git a/crates/ra_assists/src/assist_ctx.rs b/crates/ra_assists/src/assist_ctx.rs index 5924a3fd5..5aab5fb8b 100644 --- a/crates/ra_assists/src/assist_ctx.rs +++ b/crates/ra_assists/src/assist_ctx.rs @@ -1,5 +1,4 @@ //! This module defines `AssistCtx` -- the API surface that is exposed to assists. -use either::Either; use hir::{InFile, SourceAnalyzer, SourceBinder}; use ra_db::{FileRange, SourceDatabase}; use ra_fmt::{leading_indent, reindent}; @@ -11,12 +10,36 @@ use ra_syntax::{ }; use ra_text_edit::TextEditBuilder; -use crate::{AssistAction, AssistId, AssistLabel, ResolvedAssist}; +use crate::{AssistAction, AssistId, AssistLabel, GroupLabel, ResolvedAssist}; #[derive(Clone, Debug)] -pub(crate) enum Assist { - Unresolved { label: AssistLabel }, - Resolved { assist: ResolvedAssist }, +pub(crate) struct Assist(pub(crate) Vec); + +#[derive(Clone, Debug)] +pub(crate) struct AssistInfo { + pub(crate) label: AssistLabel, + pub(crate) group_label: Option, + pub(crate) action: Option, +} + +impl AssistInfo { + fn new(label: AssistLabel) -> AssistInfo { + AssistInfo { label, group_label: None, action: None } + } + + fn resolved(self, action: AssistAction) -> AssistInfo { + AssistInfo { action: Some(action), ..self } + } + + fn with_group(self, group_label: GroupLabel) -> AssistInfo { + AssistInfo { group_label: Some(group_label), ..self } + } + + pub(crate) fn into_resolved(self) -> Option { + let label = self.label; + let group_label = self.group_label; + self.action.map(|action| ResolvedAssist { label, group_label, action }) + } } pub(crate) type AssistHandler = fn(AssistCtx) -> Option; @@ -84,18 +107,17 @@ impl<'a> AssistCtx<'a> { ) -> Option { let label = AssistLabel::new(label.into(), id); - let assist = if self.should_compute_edit { + let mut info = AssistInfo::new(label); + if self.should_compute_edit { let action = { let mut edit = ActionBuilder::default(); f(&mut edit); edit.build() }; - Assist::Resolved { assist: ResolvedAssist { label, action_data: Either::Left(action) } } - } else { - Assist::Unresolved { label } + info = info.resolved(action) }; - Some(assist) + Some(Assist(vec![info])) } pub(crate) fn add_assist_group(self, group_name: impl Into) -> AssistGroup<'a> { @@ -136,7 +158,7 @@ impl<'a> AssistCtx<'a> { pub(crate) struct AssistGroup<'a> { ctx: AssistCtx<'a>, group_name: String, - assists: Vec, + assists: Vec, } impl<'a> AssistGroup<'a> { @@ -148,49 +170,22 @@ impl<'a> AssistGroup<'a> { ) { let label = AssistLabel::new(label.into(), id); - let assist = if self.ctx.should_compute_edit { + let mut info = AssistInfo::new(label).with_group(GroupLabel(self.group_name.clone())); + if self.ctx.should_compute_edit { let action = { let mut edit = ActionBuilder::default(); f(&mut edit); edit.build() }; - Assist::Resolved { assist: ResolvedAssist { label, action_data: Either::Left(action) } } - } else { - Assist::Unresolved { label } + info = info.resolved(action) }; - self.assists.push(assist) + self.assists.push(info) } pub(crate) fn finish(self) -> Option { assert!(!self.assists.is_empty()); - let mut label = match &self.assists[0] { - Assist::Unresolved { label } => label.clone(), - Assist::Resolved { assist } => assist.label.clone(), - }; - label.label = self.group_name; - let assist = if self.ctx.should_compute_edit { - Assist::Resolved { - assist: ResolvedAssist { - label, - action_data: Either::Right( - self.assists - .into_iter() - .map(|assist| match assist { - Assist::Resolved { - assist: - ResolvedAssist { label: _, action_data: Either::Left(it) }, - } => it, - _ => unreachable!(), - }) - .collect(), - ), - }, - } - } else { - Assist::Unresolved { label } - }; - Some(assist) + Some(Assist(self.assists)) } } @@ -199,7 +194,6 @@ pub(crate) struct ActionBuilder { edit: TextEditBuilder, cursor_position: Option, target: Option, - label: Option, } impl ActionBuilder { @@ -261,7 +255,6 @@ impl ActionBuilder { edit: self.edit.finish(), cursor_position: self.cursor_position, target: self.target, - label: self.label, } } } diff --git a/crates/ra_assists/src/doc_tests.rs b/crates/ra_assists/src/doc_tests.rs index ae0e5605c..c0f9bc1fb 100644 --- a/crates/ra_assists/src/doc_tests.rs +++ b/crates/ra_assists/src/doc_tests.rs @@ -30,6 +30,6 @@ fn check(assist_id: &str, before: &str, after: &str) { ) }); - let actual = assist.get_first_action().edit.apply(&before); + let actual = assist.action.edit.apply(&before); assert_eq_text!(after, &actual); } diff --git a/crates/ra_assists/src/lib.rs b/crates/ra_assists/src/lib.rs index f79189ae8..828a8e9e8 100644 --- a/crates/ra_assists/src/lib.rs +++ b/crates/ra_assists/src/lib.rs @@ -12,9 +12,6 @@ mod doc_tests; mod utils; pub mod ast_transform; -use std::cmp::Ordering; - -use either::Either; use ra_db::FileRange; use ra_ide_db::RootDatabase; use ra_syntax::{TextRange, TextUnit}; @@ -35,6 +32,9 @@ pub struct AssistLabel { pub id: AssistId, } +#[derive(Clone, Debug)] +pub struct GroupLabel(pub String); + impl AssistLabel { pub(crate) fn new(label: String, id: AssistId) -> AssistLabel { // FIXME: make fields private, so that this invariant can't be broken @@ -45,7 +45,6 @@ impl AssistLabel { #[derive(Debug, Clone)] pub struct AssistAction { - pub label: Option, pub edit: TextEdit, pub cursor_position: Option, // FIXME: This belongs to `AssistLabel` @@ -55,16 +54,8 @@ pub struct AssistAction { #[derive(Debug, Clone)] pub struct ResolvedAssist { pub label: AssistLabel, - pub action_data: Either>, -} - -impl ResolvedAssist { - pub fn get_first_action(&self) -> AssistAction { - match &self.action_data { - Either::Left(action) => action.clone(), - Either::Right(actions) => actions[0].clone(), - } - } + pub group_label: Option, + pub action: AssistAction, } /// Return all the assists applicable at the given position. @@ -76,10 +67,8 @@ pub fn unresolved_assists(db: &RootDatabase, range: FileRange) -> Vec label, - Assist::Resolved { .. } => unreachable!(), - }) + .flat_map(|it| it.0) + .map(|a| a.label) .collect() } @@ -92,24 +81,13 @@ pub fn resolved_assists(db: &RootDatabase, range: FileRange) -> Vec assist, - Assist::Unresolved { .. } => unreachable!(), - }) + .flat_map(|it| it.0) + .map(|it| it.into_resolved().unwrap()) .collect::>(); - sort_assists(&mut a); + a.sort_by_key(|it| it.action.target.map_or(TextUnit::from(!0u32), |it| it.len())); a } -fn sort_assists(assists: &mut [ResolvedAssist]) { - assists.sort_by(|a, b| match (a.get_first_action().target, b.get_first_action().target) { - (Some(a), Some(b)) => a.len().cmp(&b.len()), - (Some(_), None) => Ordering::Less, - (None, Some(_)) => Ordering::Greater, - (None, None) => Ordering::Equal, - }); -} - mod handlers { use crate::AssistHandler; @@ -184,7 +162,7 @@ mod helpers { use ra_syntax::TextRange; use test_utils::{add_cursor, assert_eq_text, extract_offset, extract_range}; - use crate::{Assist, AssistCtx, AssistHandler}; + use crate::{AssistCtx, AssistHandler}; pub(crate) fn with_single_file(text: &str) -> (RootDatabase, FileId) { let (mut db, file_id) = RootDatabase::with_single_file(text); @@ -202,10 +180,7 @@ mod helpers { FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) }; let assist = assist(AssistCtx::new(&db, frange, true)).expect("code action is not applicable"); - let action = match assist { - Assist::Unresolved { .. } => unreachable!(), - Assist::Resolved { assist } => assist.get_first_action(), - }; + let action = assist.0[0].action.clone().unwrap(); let actual = action.edit.apply(&before); let actual_cursor_pos = match action.cursor_position { @@ -225,10 +200,7 @@ mod helpers { let frange = FileRange { file_id, range }; let assist = assist(AssistCtx::new(&db, frange, true)).expect("code action is not applicable"); - let action = match assist { - Assist::Unresolved { .. } => unreachable!(), - Assist::Resolved { assist } => assist.get_first_action(), - }; + let action = assist.0[0].action.clone().unwrap(); let mut actual = action.edit.apply(&before); if let Some(pos) = action.cursor_position { @@ -244,10 +216,7 @@ mod helpers { FileRange { file_id, range: TextRange::offset_len(before_cursor_pos, 0.into()) }; let assist = assist(AssistCtx::new(&db, frange, true)).expect("code action is not applicable"); - let action = match assist { - Assist::Unresolved { .. } => unreachable!(), - Assist::Resolved { assist } => assist.get_first_action(), - }; + let action = assist.0[0].action.clone().unwrap(); let range = action.target.expect("expected target on action"); assert_eq_text!(&before[range.start().to_usize()..range.end().to_usize()], target); @@ -259,10 +228,7 @@ mod helpers { let frange = FileRange { file_id, range }; let assist = assist(AssistCtx::new(&db, frange, true)).expect("code action is not applicable"); - let action = match assist { - Assist::Unresolved { .. } => unreachable!(), - Assist::Resolved { assist } => assist.get_first_action(), - }; + let action = assist.0[0].action.clone().unwrap(); let range = action.target.expect("expected target on action"); assert_eq_text!(&before[range.start().to_usize()..range.end().to_usize()], target); -- cgit v1.2.3