diff --git a/app/gui/src/controller/graph/executed.rs b/app/gui/src/controller/graph/executed.rs index 7f4d8024ac..d6031c2177 100644 --- a/app/gui/src/controller/graph/executed.rs +++ b/app/gui/src/controller/graph/executed.rs @@ -6,6 +6,7 @@ use crate::prelude::*; +use crate::model::execution_context::ComponentGroup; use crate::model::execution_context::ComputedValueInfoRegistry; use crate::model::execution_context::LocalCall; use crate::model::execution_context::Visualization; @@ -175,6 +176,11 @@ impl Handle { self.execution_ctx.modify_visualization(id, Some(code), Some(module)).await } + /// See [`model::ExecutionContext::component_groups`]. + pub fn component_groups(&self) -> Rc> { + self.execution_ctx.component_groups() + } + /// Subscribe to updates about changes in this executed graph. /// /// The stream of notification contains both notifications from the graph and from the execution diff --git a/app/gui/src/controller/searcher.rs b/app/gui/src/controller/searcher.rs index c691906d93..2d380c26ae 100644 --- a/app/gui/src/controller/searcher.rs +++ b/app/gui/src/controller/searcher.rs @@ -8,6 +8,7 @@ use crate::controller::graph::NewNodeInfo; use crate::model::module::MethodId; use crate::model::module::NodeMetadata; use crate::model::module::Position; +use crate::model::suggestion_database; use crate::model::suggestion_database::entry::CodeToInsert; use crate::notification; @@ -477,17 +478,21 @@ impl Data { /// existing node). #[derive(Clone, CloneRef, Debug)] pub struct Searcher { - logger: Logger, - data: Rc>, - notifier: notification::Publisher, - graph: controller::ExecutedGraph, - mode: Immutable, - database: Rc, - language_server: Rc, - ide: controller::Ide, - this_arg: Rc>, + logger: Logger, + data: Rc>, + notifier: notification::Publisher, + graph: controller::ExecutedGraph, + mode: Immutable, + database: Rc, + language_server: Rc, + ide: controller::Ide, + this_arg: Rc>, position_in_code: Immutable, - project: model::Project, + project: model::Project, + /// A component list builder with favorites prepopulated with + /// [`controller::ExecutedGraph::component_groups`]. Stored to reduce the number of + /// [`database`] lookups performed when updating [`Data::components`]. + list_builder_with_favorites: Rc, } impl Searcher { @@ -534,6 +539,8 @@ impl Searcher { Mode::NewNode { source_node: Some(node), .. } => ThisNode::new(node, &graph.graph()), _ => None, }); + let favorites = graph.component_groups(); + let list_builder_with_favs = component_list_builder_with_favorites(&database, &*favorites); let ret = Self { logger, graph, @@ -546,6 +553,7 @@ impl Searcher { language_server: project.json_rpc(), position_in_code: Immutable(position), project, + list_builder_with_favorites: Rc::new(list_builder_with_favs), }; ret.reload_list(); Ok(ret) @@ -951,15 +959,15 @@ impl Searcher { let list = this.make_action_list(responses.iter()); let mut data = this.data.borrow_mut(); data.actions = Actions::Loaded { list: Rc::new(list) }; - data.components = this.make_component_list(responses.iter()); + let completions = responses.iter().flat_map(|r| r.results.iter().cloned()); + data.components = this.make_component_list(completions); } Err(err) => { let msg = "Request for completions to the Language Server returned error"; error!(this.logger, "{msg}: {err}"); let mut data = this.data.borrow_mut(); data.actions = Actions::Error(Rc::new(err.into())); - data.components = - component::List::build_list_from_all_db_entries(&this.database); + data.components = this.make_component_list(this.database.keys()); } } this.notifier.publish(Notification::NewActionList).await; @@ -1021,12 +1029,10 @@ impl Searcher { #[profile(Debug)] fn make_component_list<'a>( &self, - completion_responses: impl IntoIterator, + entry_ids: impl IntoIterator, ) -> component::List { - let mut builder = component::builder::List::new(self.database.clone_ref()); - for response in completion_responses { - builder.extend(response.results.iter().cloned()); - } + let mut builder = self.list_builder_with_favorites.deref().clone(); + builder.extend(&self.database, entry_ids); builder.build() } @@ -1158,6 +1164,21 @@ impl Searcher { } } + +// === Searcher helpers === + +fn component_list_builder_with_favorites<'a>( + suggestion_db: &model::SuggestionDatabase, + groups: impl IntoIterator, +) -> component::builder::List { + let mut builder = component::builder::List::new(); + builder.set_favorites(suggestion_db, groups); + builder +} + + +// === SimpleFunctionCall === + /// A simple function call is an AST where function is a single identifier with optional /// argument applied by `ACCESS` operator (dot). struct SimpleFunctionCall { @@ -1318,12 +1339,14 @@ pub mod test { client_setup(&mut data, &mut client); let end_of_code = enso_text::Text::from(&data.graph.module.code).location_of_text_end(); let code_range = enso_text::Location::default()..=end_of_code; + let scope = Scope::InModule { range: code_range }; let graph = data.graph.controller(); let node = &graph.graph().nodes().unwrap()[0]; let this = ThisNode::new(node.info.id(), &graph.graph()); let this = data.selected_node.and_option(this); let logger = Logger::new("Searcher"); // new_empty - let database = Rc::new(SuggestionDatabase::new_empty(&logger)); + let module_name = crate::test::mock::data::module_qualified_name(); + let database = suggestion_database_with_mock_entries(&logger, module_name, scope); let mut ide = controller::ide::MockAPI::new(); let mut project = model::project::MockAPI::new(); let project_qname = project_qualified_name(); @@ -1336,9 +1359,8 @@ pub mod test { ide.expect_current_project().returning_st(move || Some(current_project.clone_ref())); ide.expect_manage_projects() .returning_st(move || Err(ProjectOperationsNotSupported.into())); - let module_name = - QualifiedName::from_segments(data.graph.graph.project_name.clone(), &[MODULE_NAME]) - .unwrap(); + let favorites = graph.component_groups(); + let list_bldr_with_favs = component_list_builder_with_favorites(&database, &*favorites); let searcher = Searcher { graph, logger, @@ -1351,116 +1373,14 @@ pub mod test { this_arg: Rc::new(this), position_in_code: Immutable(end_of_code), project: project.clone_ref(), + list_builder_with_favorites: Rc::new(list_bldr_with_favs), }; - let entry1 = model::suggestion_database::Entry { - name: "testFunction1".to_string(), - kind: Kind::Function, - module: crate::test::mock::data::module_qualified_name(), - arguments: vec![], - return_type: "Number".to_string(), - documentation_html: default(), - self_type: None, - scope: Scope::InModule { range: code_range }, - }; - let entry2 = model::suggestion_database::Entry { - name: "TestVar1".to_string(), - kind: Kind::Local, - ..entry1.clone() - }; - let entry3 = model::suggestion_database::Entry { - name: "testMethod1".to_string(), - kind: Kind::Method, - self_type: Some(module_name.into()), - scope: Scope::Everywhere, - arguments: vec![ - Argument { - repr_type: "Any".to_string(), - name: "this".to_string(), - has_default: false, - default_value: None, - is_suspended: false, - }, - Argument { - repr_type: "Number".to_string(), - name: "num_arg".to_string(), - has_default: false, - default_value: None, - is_suspended: false, - }, - ], - ..entry1.clone() - }; - let entry4 = model::suggestion_database::Entry { - self_type: Some("test.Test.Test".to_owned().try_into().unwrap()), - module: "test.Test.Test".to_owned().try_into().unwrap(), - arguments: vec![ - Argument { - repr_type: "Any".to_string(), - name: "this".to_string(), - has_default: false, - default_value: None, - is_suspended: false, - }, - Argument { - repr_type: "String".to_string(), - name: "num_arg".to_string(), - has_default: false, - default_value: None, - is_suspended: false, - }, - ], - ..entry3.clone() - }; - let entry5 = model::suggestion_database::Entry { - kind: Kind::Module, - module: entry1.module.clone(), - name: MODULE_NAME.to_owned(), - arguments: default(), - return_type: entry1.module.to_string(), - documentation_html: None, - self_type: None, - scope: Scope::Everywhere, - }; - let entry9 = model::suggestion_database::Entry { - name: "testFunction2".to_string(), - arguments: vec![ - Argument { - repr_type: "Text".to_string(), - name: "text_arg".to_string(), - has_default: false, - default_value: None, - is_suspended: false, - }, - Argument { - repr_type: "Number".to_string(), - name: "num_arg".to_string(), - has_default: false, - default_value: None, - is_suspended: false, - }, - ], - ..entry1.clone() - }; - let entry10 = model::suggestion_database::Entry { - name: "testFunction3".to_string(), - module: "test.Test.Other".to_owned().try_into().unwrap(), - scope: Scope::Everywhere, - ..entry9.clone() - }; - - searcher.database.put_entry(1, entry1); let entry1 = searcher.database.lookup(1).unwrap(); - searcher.database.put_entry(2, entry2); let entry2 = searcher.database.lookup(2).unwrap(); - searcher.database.put_entry(3, entry3); let entry3 = searcher.database.lookup(3).unwrap(); - searcher.database.put_entry(4, entry4); let entry4 = searcher.database.lookup(4).unwrap(); - searcher.database.put_entry(5, entry5); let entry5 = searcher.database.lookup(5).unwrap(); - searcher.database.put_entry(9, entry9); let entry9 = searcher.database.lookup(9).unwrap(); - searcher.database.put_entry(10, entry10); let entry10 = searcher.database.lookup(10).unwrap(); Fixture { data, @@ -1481,6 +1401,117 @@ pub mod test { } } + fn suggestion_database_with_mock_entries( + logger: &Logger, + module_name: QualifiedName, + scope: Scope, + ) -> Rc { + let database = Rc::new(SuggestionDatabase::new_empty(logger)); + let entry1 = model::suggestion_database::Entry { + name: "testFunction1".to_string(), + kind: Kind::Function, + module: crate::test::mock::data::module_qualified_name(), + arguments: vec![], + return_type: "Number".to_string(), + documentation_html: default(), + self_type: None, + scope, + }; + let entry2 = model::suggestion_database::Entry { + name: "TestVar1".to_string(), + kind: Kind::Local, + ..entry1.clone() + }; + let entry3 = model::suggestion_database::Entry { + name: "testMethod1".to_string(), + kind: Kind::Method, + self_type: Some(module_name.into()), + scope: Scope::Everywhere, + arguments: vec![ + Argument { + repr_type: "Any".to_string(), + name: "this".to_string(), + has_default: false, + default_value: None, + is_suspended: false, + }, + Argument { + repr_type: "Number".to_string(), + name: "num_arg".to_string(), + has_default: false, + default_value: None, + is_suspended: false, + }, + ], + ..entry1.clone() + }; + let entry4 = model::suggestion_database::Entry { + self_type: Some("test.Test.Test".to_owned().try_into().unwrap()), + module: "test.Test.Test".to_owned().try_into().unwrap(), + arguments: vec![ + Argument { + repr_type: "Any".to_string(), + name: "this".to_string(), + has_default: false, + default_value: None, + is_suspended: false, + }, + Argument { + repr_type: "String".to_string(), + name: "num_arg".to_string(), + has_default: false, + default_value: None, + is_suspended: false, + }, + ], + ..entry3.clone() + }; + let entry5 = model::suggestion_database::Entry { + kind: Kind::Module, + module: entry1.module.clone(), + name: MODULE_NAME.to_owned(), + arguments: default(), + return_type: entry1.module.to_string(), + documentation_html: None, + self_type: None, + scope: Scope::Everywhere, + }; + let entry9 = model::suggestion_database::Entry { + name: "testFunction2".to_string(), + arguments: vec![ + Argument { + repr_type: "Text".to_string(), + name: "text_arg".to_string(), + has_default: false, + default_value: None, + is_suspended: false, + }, + Argument { + repr_type: "Number".to_string(), + name: "num_arg".to_string(), + has_default: false, + default_value: None, + is_suspended: false, + }, + ], + ..entry1.clone() + }; + let entry10 = model::suggestion_database::Entry { + name: "testFunction3".to_string(), + module: "test.Test.Other".to_owned().try_into().unwrap(), + scope: Scope::Everywhere, + ..entry9.clone() + }; + database.put_entry(1, entry1); + database.put_entry(2, entry2); + database.put_entry(3, entry3); + database.put_entry(4, entry4); + database.put_entry(5, entry5); + database.put_entry(9, entry9); + database.put_entry(10, entry10); + database + } + /// Test checks that: /// 1) if the selected node is assigned to a single variable (or can be assigned), the list is @@ -1673,14 +1704,30 @@ pub mod test { #[wasm_bindgen_test] fn loading_components() { + // Prepare a sample component group to be returned by a mock Language Server client. + let module_qualified_name = crate::test::mock::data::module_qualified_name().to_string(); + let sample_ls_component_group = language_server::LibraryComponentGroup { + library: "".to_string(), + name: "Test Group 1".to_string(), + color: None, + icon: None, + exports: vec![language_server::LibraryComponent { + name: module_qualified_name + ".testFunction1", + shortcut: None, + }], + }; + // Create a test fixture with mocked Engine responses. let Fixture { mut test, searcher, entry1, entry9, .. } = Fixture::new_custom(|data, client| { // Entry with id 99999 does not exist, so only two actions from suggestions db // should be displayed in searcher. data.expect_completion(client, None, None, &[1, 99999, 9]); + data.graph.ctx.component_groups = vec![sample_ls_component_group]; }); + // Reload the components list in the Searcher. searcher.reload_list(); test.run_until_stalled(); + // Verify the contents of the components list loaded by the Searcher. let components = searcher.components(); if let [module_group] = &components.top_modules()[..] { assert_eq!(module_group.name, entry1.module.to_string()); @@ -1689,6 +1736,13 @@ pub mod test { } else { ipanic!("Wrong top modules in Component List: {components.top_modules():?}"); } + let favorites = &components.favorites; + assert_eq!(favorites.len(), 1); + let favorites_group = &favorites[0]; + assert_eq!(favorites_group.name, "Test Group 1"); + let favorites_entries = favorites_group.entries.borrow(); + assert_eq!(favorites_entries.len(), 1); + assert_eq!(*favorites_entries[0].id, 1); } #[wasm_bindgen_test] diff --git a/app/gui/src/controller/searcher/component.rs b/app/gui/src/controller/searcher/component.rs index a4dd035816..9ba23b6997 100644 --- a/app/gui/src/controller/searcher/component.rs +++ b/app/gui/src/controller/searcher/component.rs @@ -105,7 +105,7 @@ impl Component { #[derive(Clone, CloneRef, Debug)] pub struct ModuleGroups { pub content: Group, - pub submodules: group::List, + pub submodules: group::AlphabeticalList, } @@ -124,24 +124,20 @@ pub struct ModuleGroups { #[derive(Clone, CloneRef, Debug, Default)] pub struct List { all_components: Rc>, - top_modules: group::List, - top_modules_flattened: group::List, + top_modules: group::AlphabeticalList, + top_modules_flattened: group::AlphabeticalList, module_groups: Rc>, filtered: Rc>, + /// Groups of components to display in the "Favorites Data Science Tools" section of the + /// [Component Browser](crate::controller::Searcher). + pub favorites: group::List, } impl List { - /// Create a list containing all entities available in the [`model::SuggestionDatabase`]. - pub fn build_list_from_all_db_entries(suggestion_db: &Rc) -> List { - let mut builder = builder::List::new(suggestion_db.clone_ref()); - builder.extend(suggestion_db.keys()); - builder.build() - } - /// Return the list of top modules, which should be displayed in Component Browser. /// /// If the list is filtered, all top modules will be flattened. - pub fn top_modules(&self) -> &group::List { + pub fn top_modules(&self) -> &group::AlphabeticalList { if self.filtered.get() { &self.top_modules_flattened } else { @@ -151,7 +147,7 @@ impl List { /// Get the list of given component submodules. Returns [`None`] if given component is not /// a module. - pub fn submodules_of(&self, component: Id) -> Option<&group::List> { + pub fn submodules_of(&self, component: Id) -> Option<&group::AlphabeticalList> { self.module_groups.get(&component).map(|mg| &mg.submodules) } @@ -167,13 +163,17 @@ impl List { for component in &*self.all_components { component.update_matching_info(pattern) } - for group in self.all_groups() { - group.update_sorting(pattern); + for group in self.all_groups_not_in_favorites() { + group.update_sorting_and_visibility(pattern); + } + for group in self.favorites.iter() { + group.update_visibility(); } self.filtered.set(!pattern.is_empty()); } - fn all_groups(&self) -> impl Iterator { + /// All groups from [`List`] without the groups found in [`List::favorites`]. + fn all_groups_not_in_favorites(&self) -> impl Iterator { let normal = self.module_groups.values().map(|mg| &mg.content); let flattened = self.top_modules_flattened.iter(); normal.chain(flattened) @@ -257,9 +257,37 @@ pub(crate) mod tests { suggestion_db } + fn mock_favorites( + db: &model::SuggestionDatabase, + component_ids: &[Id], + ) -> Vec { + let db_entries = component_ids.iter().map(|id| db.lookup(*id).unwrap()); + let group = crate::model::execution_context::ComponentGroup { + name: "Test Group 1".into(), + color: None, + components: db_entries.into_iter().map(|e| e.qualified_name()).collect(), + }; + vec![group] + } + // === Filtering Component List === + /// Assert IDs and order of all entries in the group which have their [`Component::match_info`] + /// set to [`MatchInfo::Matches`]. Additionally, verify the [`Group::visible`] field is + /// [`true`] iff no IDs are expected. + fn assert_ids_of_matches_entries(group: &Group, expected_ids: &[Id]) { + let ids_of_matches = group + .entries + .borrow() + .iter() + .filter(|c| matches!(*c.match_info.borrow(), MatchInfo::Matches { .. })) + .map(|c| *c.id) + .collect_vec(); + assert_eq!(ids_of_matches, expected_ids); + assert_eq!(group.visible.get(), !expected_ids.is_empty()); + } + #[test] fn filtering_component_list() { let logger = Logger::new("test::update_list_after_filtering_pattern_change"); @@ -267,45 +295,36 @@ pub(crate) mod tests { let sub_module = mock_module("test.Test.TopModule.SubModule"); let fun1 = mock_function(&top_module.module, "fun1"); let funx2 = mock_function(&sub_module.module, "funx1"); - let all_entries = [top_module, sub_module, fun1, funx2]; + let all_entries = [&top_module, &sub_module, &fun1, &funx2]; let suggestion_db = model::SuggestionDatabase::new_empty(logger); for (id, entry) in all_entries.into_iter().enumerate() { - suggestion_db.put_entry(id, entry) + suggestion_db.put_entry(id, entry.clone()) } - let mut builder = builder::List::new(Rc::new(suggestion_db)); - builder.extend(0..4); + let favorites = mock_favorites(&suggestion_db, &[3, 2]); + let mut builder = builder::List::new(); + builder.set_favorites(&suggestion_db, &favorites); + builder.extend(&suggestion_db, 0..4); let list = builder.build(); - let get_entries_ids = - || list.top_modules()[0].entries.borrow().iter().map(|c| *c.id).collect_vec(); - let count_matches_entries = || { - list.top_modules()[0] - .entries - .borrow() - .iter() - .take_while(|c| matches!(*c.match_info.borrow(), MatchInfo::Matches { .. })) - .count() - }; list.update_filtering("fu"); - let expected_ids = vec![2, 3, 1]; - assert_eq!(get_entries_ids(), expected_ids); - assert_eq!(count_matches_entries(), 2); - assert!(list.top_modules()[0].visible.get()); + assert_ids_of_matches_entries(&list.top_modules()[0], &[2, 3]); + assert_ids_of_matches_entries(&list.favorites[0], &[3, 2]); list.update_filtering("x"); - let expected_ids = vec![3, 2, 1]; - assert_eq!(get_entries_ids(), expected_ids); - assert_eq!(count_matches_entries(), 1); - assert!(list.top_modules()[0].visible.get()); + assert_ids_of_matches_entries(&list.top_modules()[0], &[3]); + assert_ids_of_matches_entries(&list.favorites[0], &[3]); list.update_filtering("Sub"); - let expected_ids = vec![1, 3, 2]; - assert_eq!(get_entries_ids(), expected_ids); - assert_eq!(count_matches_entries(), 1); - assert!(list.top_modules()[0].visible.get()); + assert_ids_of_matches_entries(&list.top_modules()[0], &[1]); + assert_ids_of_matches_entries(&list.favorites[0], &[]); list.update_filtering("y"); - assert!(!list.top_modules()[0].visible.get()); + assert_ids_of_matches_entries(&list.top_modules()[0], &[]); + assert_ids_of_matches_entries(&list.favorites[0], &[]); + + list.update_filtering(""); + assert_ids_of_matches_entries(&list.top_modules()[0], &[2, 1]); + assert_ids_of_matches_entries(&list.favorites[0], &[3, 2]); } @@ -316,8 +335,8 @@ pub(crate) mod tests { // Create a components list with sample data. let logger = Logger::new("test::component_list_modules_tree"); let suggestion_db = mock_suggestion_db(logger); - let mut builder = builder::List::new(Rc::new(suggestion_db)); - builder.extend(0..11); + let mut builder = builder::List::new(); + builder.extend(&suggestion_db, 0..11); let list = builder.build(); // Verify that we can read all top-level modules from the component list. diff --git a/app/gui/src/controller/searcher/component/builder.rs b/app/gui/src/controller/searcher/component/builder.rs index 6f8e94fb9c..33ec61f53c 100644 --- a/app/gui/src/controller/searcher/component/builder.rs +++ b/app/gui/src/controller/searcher/component/builder.rs @@ -4,6 +4,7 @@ use crate::prelude::*; use crate::controller::searcher::component; use crate::controller::searcher::component::Component; +use crate::model::execution_context; use crate::model::suggestion_database; use double_representation::module; @@ -27,7 +28,7 @@ pub struct ModuleGroups { /// For example when the module is a top module, so need its flattened content to fill the /// `top_module_flattened` field of [`component::List`]. pub flattened_content: Option, - pub submodules: component::group::ListBuilder, + pub submodules: component::group::AlphabeticalListBuilder, pub is_top_module: bool, } @@ -62,36 +63,35 @@ impl ModuleGroups { /// /// The builder allow extending the list with new entries, and build a list with properly sorted /// groups. -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Default)] pub struct List { - suggestion_db: Rc, all_components: Vec, module_groups: HashMap, + favorites: component::group::List, } impl List { /// Construct List builder without content. - /// - /// The given suggestion_db will be used to look up entries when extending (the [`Self::extend`] - /// method takes ids as argument). - pub fn new(suggestion_db: Rc) -> Self { - Self { suggestion_db, all_components: default(), module_groups: default() } + pub fn new() -> Self { + default() } - /// Extend the list with new entries. - pub fn extend(&mut self, entries: impl IntoIterator) { - let suggestion_db = self.suggestion_db.clone_ref(); - let components = entries - .into_iter() - .filter_map(|id| Some(Component::new(id, suggestion_db.lookup(id).ok()?))); + /// Extend the list with new entries looked up by ID in suggestion database. + pub fn extend( + &mut self, + db: &model::SuggestionDatabase, + entries: impl IntoIterator, + ) { + let lookup_component_by_id = |id| Some(Component::new(id, db.lookup(id).ok()?)); + let components = entries.into_iter().filter_map(lookup_component_by_id); for component in components { let mut component_inserted_somewhere = false; if let Some(parent_module) = component.suggestion.parent_module() { - if let Some(parent_group) = self.lookup_module_group(&parent_module) { + if let Some(parent_group) = self.lookup_module_group(db, &parent_module) { parent_group.content.entries.borrow_mut().push(component.clone_ref()); component_inserted_somewhere = true; } - if let Some(top_group) = self.lookup_module_group(&parent_module.top_module()) { + if let Some(top_group) = self.lookup_module_group(db, &parent_module.top_module()) { if let Some(flatten_group) = &mut top_group.flattened_content { flatten_group.entries.borrow_mut().push(component.clone_ref()); component_inserted_somewhere = true; @@ -104,8 +104,27 @@ impl List { } } - fn lookup_module_group(&mut self, module: &module::QualifiedName) -> Option<&mut ModuleGroups> { - let (module_id, db_entry) = self.suggestion_db.lookup_by_qualified_name(module)?; + /// Set the favorites in the list. Components are looked up by ID in the suggestion database. + pub fn set_favorites<'a>( + &mut self, + db: &model::SuggestionDatabase, + component_groups: impl IntoIterator, + ) { + self.favorites = component_groups + .into_iter() + .filter_map(|g| component::Group::from_execution_context_component_group(g, db)) + .collect(); + for group in &*self.favorites { + self.all_components.extend(group.entries.borrow().iter().cloned()); + } + } + + fn lookup_module_group( + &mut self, + db: &model::SuggestionDatabase, + module: &module::QualifiedName, + ) -> Option<&mut ModuleGroups> { + let (module_id, db_entry) = db.lookup_by_qualified_name(module)?; // Note: My heart is bleeding at this point, but because of lifetime checker limitations // we must do it in this suboptimal way. @@ -117,7 +136,7 @@ impl List { } else { let groups = ModuleGroups::new(module_id, &*db_entry); if let Some(module) = module.parent_module() { - if let Some(parent_groups) = self.lookup_module_group(&module) { + if let Some(parent_groups) = self.lookup_module_group(db, &module) { parent_groups.submodules.push(groups.content.clone_ref()) } } @@ -125,18 +144,19 @@ impl List { } } - /// Build the list, sorting all group lists and groups' contents appropriately. + /// Build the list, sorting all group lists and groups' contents appropriately. (Does not sort + /// the [`component::List::favorites`].) pub fn build(self) -> component::List { for group in self.module_groups.values() { - group.content.update_sorting(""); + group.content.update_sorting_and_visibility(""); if let Some(flattened) = &group.flattened_content { - flattened.update_sorting(""); + flattened.update_sorting_and_visibility(""); } } let top_modules_iter = self.module_groups.values().filter(|g| g.is_top_module); - let mut top_mdl_bld = component::group::ListBuilder::default(); + let mut top_mdl_bld = component::group::AlphabeticalListBuilder::default(); top_mdl_bld.extend(top_modules_iter.clone().map(|g| g.content.clone_ref())); - let mut top_mdl_flat_bld = component::group::ListBuilder::default(); + let mut top_mdl_flat_bld = component::group::AlphabeticalListBuilder::default(); top_mdl_flat_bld.extend(top_modules_iter.filter_map(|g| g.flattened_content.clone())); component::List { all_components: Rc::new(self.all_components), @@ -146,6 +166,7 @@ impl List { self.module_groups.into_iter().map(|(id, group)| (id, group.build())).collect(), ), filtered: default(), + favorites: self.favorites, } } } @@ -184,11 +205,11 @@ mod tests { fn building_component_list() { let logger = Logger::new("tests::module_groups_in_component_list"); let suggestion_db = Rc::new(mock_suggestion_db(logger)); - let mut builder = List::new(suggestion_db); + let mut builder = List::new(); let first_part = (0..3).chain(6..11); let second_part = 3..6; - builder.extend(first_part); - builder.extend(second_part); + builder.extend(&suggestion_db, first_part); + builder.extend(&suggestion_db, second_part); let list = builder.build(); let top_modules: Vec = diff --git a/app/gui/src/controller/searcher/component/group.rs b/app/gui/src/controller/searcher/component/group.rs index 06840baf10..17dbe874d5 100644 --- a/app/gui/src/controller/searcher/component/group.rs +++ b/app/gui/src/controller/searcher/component/group.rs @@ -5,8 +5,11 @@ use crate::prelude::*; use crate::controller::searcher::component; use crate::controller::searcher::component::Component; +use crate::model::execution_context; use crate::model::suggestion_database; +use ensogl::data::color; + // ============ @@ -18,6 +21,7 @@ use crate::model::suggestion_database; #[derive(Clone, Debug)] pub struct Data { pub name: ImString, + pub color: Option, /// A component corresponding to this group, e.g. the module of whose content the group /// contains. pub component_id: Option, @@ -29,7 +33,13 @@ pub struct Data { impl Data { fn new_empty_visible(name: impl Into, component_id: Option) -> Self { - Data { name: name.into(), component_id, entries: default(), visible: Cell::new(true) } + Data { + name: name.into(), + color: None, + component_id, + entries: default(), + visible: Cell::new(true), + } } } @@ -63,19 +73,59 @@ impl Group { Self { data: Rc::new(Data::new_empty_visible(name, Some(component_id))) } } - /// Update the group sorting according to the current filtering pattern. - pub fn update_sorting(&self, pattern: impl AsRef) { - let mut entries = self.entries.borrow_mut(); - // The `sort_by_key` method is not suitable here, because the closure it takes - // cannot return reference nor [`Ref`], and we don't want to copy anything here. - if pattern.as_ref().is_empty() { - entries.sort_by(|a, b| { - a.can_be_entered().cmp(&b.can_be_entered()).then_with(|| a.label().cmp(b.label())) - }); - } else { - entries.sort_by(|a, b| a.match_info.borrow().cmp(&*b.match_info.borrow()).reverse()); + /// Construct from [`execution_context::ComponentGroup`] components looked up in the suggestion + /// database by their full qualified name. Returns a group containing only the successfully + /// looked up components, or [`None`] if none of the components were found in the suggestion + /// database. + pub fn from_execution_context_component_group( + group: &execution_context::ComponentGroup, + suggestion_db: &model::SuggestionDatabase, + ) -> Option { + let lookup_component = |qualified_name| { + let (id, suggestion) = suggestion_db.lookup_by_qualified_name(qualified_name)?; + Some(Component::new(id, suggestion)) + }; + let components = &group.components; + let looked_up_components = components.iter().filter_map(lookup_component).collect_vec(); + let any_components_found_in_db = !looked_up_components.is_empty(); + any_components_found_in_db.then(|| { + let group_data = Data { + name: group.name.clone(), + color: group.color, + component_id: None, + visible: Cell::new(true), + entries: RefCell::new(looked_up_components), + }; + Group { data: Rc::new(group_data) } + }) + } + + /// Update the group sorting according to the current filtering pattern and call + /// [`update_visibility`]. + pub fn update_sorting_and_visibility(&self, pattern: impl AsRef) { + { + let mut entries = self.entries.borrow_mut(); + // The `sort_by_key` method is not suitable here, because the closure it takes + // cannot return reference nor [`Ref`], and we don't want to copy anything here. + if pattern.as_ref().is_empty() { + entries.sort_by(|a, b| { + let cmp_can_be_entered = a.can_be_entered().cmp(&b.can_be_entered()); + cmp_can_be_entered.then_with(|| a.label().cmp(b.label())) + }); + } else { + let cmp_match_info = |a: &Component, b: &Component| { + a.match_info.borrow().cmp(&*b.match_info.borrow()) + }; + entries.sort_by(|a, b| cmp_match_info(a, b).reverse()); + } } - let visible = !entries.iter().all(|c| c.is_filtered_out()); + self.update_visibility(); + } + + /// Sets the [`visible`] flag to [`true`] if at least one of the group's entries is not + /// filtered out. Sets the flag to [`false`] otherwise. + pub fn update_visibility(&self) { + let visible = !self.entries.borrow().iter().all(|c| c.is_filtered_out()); self.visible.set(visible); } } @@ -86,12 +136,25 @@ impl Group { // === List === // ============ -/// An immutable [`Group`] list, keeping the groups in alphabetical order. +/// An immutable [`Group`] list, keeping the groups in the order provided in the constructor. #[derive(Clone, CloneRef, Debug, Default)] pub struct List { groups: Rc>, } +impl List { + /// Constructor. + pub fn new(groups: Vec) -> Self { + Self { groups: Rc::new(groups) } + } +} + +impl FromIterator for List { + fn from_iter>(iter: T) -> Self { + Self::new(iter.into_iter().collect()) + } +} + impl Deref for List { type Target = [Group]; fn deref(&self) -> &Self::Target { @@ -106,22 +169,99 @@ impl AsRef<[Group]> for List { } -// === ListBuilder === + +// ======================== +// === AlphabeticalList === +// ======================== + +/// An immutable [`Group`] list, keeping the groups in alphabetical order. +#[derive(Clone, CloneRef, Debug, Default, AsRef, Deref)] +pub struct AlphabeticalList { + groups: List, +} -/// The builder of [`List`]. The groups will be sorted in [`Self::build`] method. +// === AlphabeticalListBuilder === + +/// The builder of [`AlphabeticalList`]. The groups will be sorted in [`Self::build`] method. #[allow(missing_docs)] #[derive(Clone, Debug, Default, AsRef, Deref, AsMut, DerefMut)] -pub struct ListBuilder { +pub struct AlphabeticalListBuilder { pub groups: Vec, } -impl ListBuilder { - /// Sort the groups and create a [`List`]. - pub fn build(mut self) -> List { +impl AlphabeticalListBuilder { + /// Sort the groups and create an [`AlphabeticalList`]. + pub fn build(mut self) -> AlphabeticalList { // The `sort_unstable_by_key` method is not suitable here, because the closure it takes // cannot return reference, and we don't want to copy strings here. self.groups.sort_unstable_by(|a, b| a.name.cmp(&b.name)); - List { groups: Rc::new(self.groups) } + AlphabeticalList { groups: List::new(self.groups) } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + use crate::controller::searcher::component::tests::mock_suggestion_db; + use std::assert_matches::assert_matches; + + /// Test whether [`Group::from_execution_context_component_group`] correctly looks up + /// components in the suggestion database. + #[test] + fn lookup_component_groups_in_suggestion_database() { + let logger = Logger::new("tests::lookup_component_groups_in_suggestion_database"); + let suggestion_db = Rc::new(mock_suggestion_db(logger)); + + // Prepare a mock group containing fully qualified component names in non-alphabetical + // order. Some of the names correspond to entries present in the suggestion database, + // some do not. + let ec_group = execution_context::ComponentGroup { + name: "Test Group 1".into(), + color: color::Rgb::from_css_hex("#aabbcc"), + components: vec![ + "test.Test.TopModule1.fun2".into(), + "test.Test.TopModule1.SubModule2.SubModule3.fun6".into(), + "test.Test.NonExistantModule.fun6".into(), + "test.Test.TopModule1.fun1".into(), + "test.Test.TopModule1.nonexistantfun".into(), + ], + }; + + // Construct a components group with entries looked up in the suggestion database. + let group = Group::from_execution_context_component_group(&ec_group, &suggestion_db); + + // Verify the contents of the components group. + let group = group.unwrap(); + assert_eq!(group.name, ImString::new("Test Group 1")); + let color = group.color.unwrap(); + assert_eq!((color.red * 255.0) as u8, 0xaa); + assert_eq!((color.green * 255.0) as u8, 0xbb); + assert_eq!((color.blue * 255.0) as u8, 0xcc); + let entry_ids_and_names = group + .entries + .borrow() + .iter() + .map(|e| (*e.id, e.suggestion.name.to_string())) + .collect_vec(); + let expected_ids_and_names = + vec![(6, "fun2".to_string()), (10, "fun6".to_string()), (5, "fun1".to_string())]; + assert_eq!(entry_ids_and_names, expected_ids_and_names); + } + + // Test constructing a component group from an [`execution_context::ComponentGroup`] containing + // only names not found in the suggestion database. + #[test] + fn constructing_component_group_from_names_not_found_in_db() { + let logger = Logger::new("tests::constructing_component_group_from_names_not_found_in_db"); + let suggestion_db = Rc::new(mock_suggestion_db(logger)); + let ec_group = execution_context::ComponentGroup { + name: "Input".into(), + color: None, + components: vec!["NAME.NOT.FOUND.IN.DB".into()], + }; + let group = Group::from_execution_context_component_group(&ec_group, &suggestion_db); + assert_matches!(group, None); } } diff --git a/app/gui/src/lib.rs b/app/gui/src/lib.rs index f32045d243..948fb1162a 100644 --- a/app/gui/src/lib.rs +++ b/app/gui/src/lib.rs @@ -29,7 +29,6 @@ #![recursion_limit = "512"] // === Features === -#![feature(arbitrary_self_types)] #![feature(async_closure)] #![feature(associated_type_bounds)] #![feature(bool_to_option)] diff --git a/app/gui/src/model/execution_context.rs b/app/gui/src/model/execution_context.rs index d14bff080b..53ad8827c8 100644 --- a/app/gui/src/model/execution_context.rs +++ b/app/gui/src/model/execution_context.rs @@ -339,6 +339,11 @@ pub trait API: Debug { /// Returns IDs of all active visualizations. fn active_visualizations(&self) -> Vec; + /// Get the component groups defined in libraries imported into the execution context, or an + /// empty vector if component groups are not yet loaded. Component groups are loaded after the + /// execution context becomes ready and a response from the Engine is received. + fn component_groups(&self) -> Rc>; + /// Get the registry of computed values. fn computed_value_info_registry(&self) -> &Rc; diff --git a/app/gui/src/model/execution_context/plain.rs b/app/gui/src/model/execution_context/plain.rs index f69d4bd327..d92e14804a 100644 --- a/app/gui/src/model/execution_context/plain.rs +++ b/app/gui/src/model/execution_context/plain.rs @@ -61,7 +61,7 @@ pub struct ExecutionContext { /// Execution context is considered ready once it completes it first execution after creation. pub is_ready: crate::sync::Synchronized, /// Component groups defined in libraries imported into the execution context. - pub component_groups: RefCell>, + pub component_groups: RefCell>>, } impl ExecutionContext { @@ -186,6 +186,10 @@ impl model::execution_context::API for ExecutionContext { self.visualizations.borrow().keys().copied().collect_vec() } + fn component_groups(&self) -> Rc> { + self.component_groups.borrow().clone() + } + fn computed_value_info_registry(&self) -> &Rc { &self.computed_value_info_registry } @@ -308,9 +312,16 @@ pub mod test { } } + fn component_groups(&self) -> RefCell>> { + let groups = self.component_groups.iter().map(|g| g.clone().into()).collect(); + RefCell::new(Rc::new(groups)) + } + pub fn create(&self) -> ExecutionContext { let logger = Logger::new("Mocked Execution Context"); - ExecutionContext::new(logger, self.main_method_pointer()) + let mut ec = ExecutionContext::new(logger, self.main_method_pointer()); + ec.component_groups = self.component_groups(); + ec } } } diff --git a/app/gui/src/model/execution_context/synchronized.rs b/app/gui/src/model/execution_context/synchronized.rs index 9763590554..b0186a2bf2 100644 --- a/app/gui/src/model/execution_context/synchronized.rs +++ b/app/gui/src/model/execution_context/synchronized.rs @@ -2,6 +2,7 @@ use crate::prelude::*; +use crate::model::execution_context::ComponentGroup; use crate::model::execution_context::ComputedValueInfoRegistry; use crate::model::execution_context::LocalCall; use crate::model::execution_context::Visualization; @@ -77,16 +78,6 @@ impl ExecutionContext { let this = Self { id, model, language_server, logger }; this.push_root_frame().await?; info!(this.logger, "Pushed root frame."); - match this.load_component_groups().await { - Ok(_) => info!(this.logger, "Loaded component groups."), - Err(err) => { - let msg = iformat!( - "Failed to load component groups. No groups will appear in the Favorites \ - section of the Component Browser. Error: {err}" - ); - error!(this.logger, "{msg}"); - } - } Ok(this) } } @@ -107,11 +98,22 @@ impl ExecutionContext { } /// Load the component groups defined in libraries imported into the execution context. - async fn load_component_groups(&self) -> FallibleResult { - let ls_response = self.language_server.get_component_groups(&self.id).await?; - *self.model.component_groups.borrow_mut() = - ls_response.component_groups.into_iter().map(|group| group.into()).collect(); - Ok(()) + async fn load_component_groups(&self) { + match self.language_server.get_component_groups(&self.id).await { + Ok(ls_response) => { + let ls_groups = ls_response.component_groups; + let groups = ls_groups.into_iter().map(|group| group.into()).collect(); + *self.model.component_groups.borrow_mut() = Rc::new(groups); + info!(self.logger, "Loaded component groups."); + } + Err(err) => { + let msg = iformat!( + "Failed to load component groups. No groups will appear in the Favorites \ + section of the Component Browser. Error: {err}" + ); + error!(self.logger, "{msg}"); + } + } } /// Detach visualization from current execution context. @@ -136,11 +138,15 @@ impl ExecutionContext { } /// Handles the update about expressions being computed. - pub fn handle_notification(&self, notification: Notification) -> FallibleResult { + pub fn handle_notification(self: &Rc, notification: Notification) -> FallibleResult { match notification { Notification::Completed => if !self.model.is_ready.replace(true) { info!(self.logger, "Context {self.id} Became ready"); + let this = self.clone(); + executor::global::spawn(async move { + this.load_component_groups().await; + }); }, Notification::ExpressionUpdates(updates) => { self.model.computed_value_info_registry.apply_updates(updates); @@ -171,6 +177,10 @@ impl model::execution_context::API for ExecutionContext { self.model.active_visualizations() } + fn component_groups(&self) -> Rc> { + self.model.component_groups() + } + /// Access the registry of computed values information, like types or called method pointers. fn computed_value_info_registry(&self) -> &Rc { self.model.computed_value_info_registry() @@ -372,10 +382,6 @@ pub mod test { }; let stack_item = language_server::StackItem::ExplicitCall(root_frame); expect_call!(ls.push_to_execution_context(id,stack_item) => Ok(())); - let component_groups = language_server::response::GetComponentGroups { - component_groups: data.component_groups.clone(), - }; - expect_call!(ls.get_component_groups(id) => Ok(component_groups)); } /// Generates a mock update for a random expression id. @@ -573,14 +579,19 @@ pub mod test { ]; // Create a test fixture based on the sample data. - let mut mock_data = MockData::new(); - mock_data.component_groups = sample_ls_component_groups; - let fixture = Fixture::new_customized_with_data(mock_data, |_, _| {}); + let fixture = Fixture::new_customized(move |client, data| { + let component_groups = language_server::response::GetComponentGroups { + component_groups: sample_ls_component_groups, + }; + let id = data.context_id; + expect_call!(client.get_component_groups(id) => Ok(component_groups)); + }); let Fixture { mut test, context, .. } = fixture; // Run a test and verify that the sample component groups were parsed correctly and have // expected contents. test.run_task(async move { + context.load_component_groups().await; let groups = context.model.component_groups.borrow(); assert_eq!(groups.len(), 2);