diff --git a/app/gui/src/controller/searcher.rs b/app/gui/src/controller/searcher.rs index 4ec889655a..ec5114326b 100644 --- a/app/gui/src/controller/searcher.rs +++ b/app/gui/src/controller/searcher.rs @@ -1042,7 +1042,7 @@ impl Searcher { entry_ids: impl IntoIterator, ) -> component::List { let mut builder = self.list_builder_with_favorites.deref().clone(); - builder.extend(&self.database, entry_ids); + builder.extend_list_and_allow_favorites_with_ids(&self.database, entry_ids); builder.build() } @@ -1186,7 +1186,7 @@ fn component_list_builder_with_favorites<'a>( if let Some((id, _)) = suggestion_db.lookup_by_qualified_name(local_scope_module) { builder = builder.with_local_scope_module_id(id); } - builder.set_favorites(suggestion_db, groups); + builder.set_grouping_and_order_of_favorites(suggestion_db, groups); builder } @@ -1787,10 +1787,16 @@ pub mod test { name: "Test Group 1".to_string(), color: None, icon: None, - exports: vec![language_server::LibraryComponent { - name: module_qualified_name + ".testFunction1", - shortcut: None, - }], + exports: vec![ + language_server::LibraryComponent { + name: module_qualified_name.clone() + ".testFunction1", + shortcut: None, + }, + language_server::LibraryComponent { + name: module_qualified_name + ".testMethod1", + shortcut: None, + }, + ], }; // Create a test fixture with mocked Engine responses. let Fixture { mut test, searcher, entry1, entry9, .. } = diff --git a/app/gui/src/controller/searcher/component.rs b/app/gui/src/controller/searcher/component.rs index e5ffa87c61..d166e0dabd 100644 --- a/app/gui/src/controller/searcher/component.rs +++ b/app/gui/src/controller/searcher/component.rs @@ -400,8 +400,8 @@ pub(crate) mod tests { } let favorites = mock_favorites(&suggestion_db, &[3, 2]); let mut builder = builder::List::new().with_local_scope_module_id(0); - builder.set_favorites(&suggestion_db, &favorites); - builder.extend(&suggestion_db, 0..4); + builder.set_grouping_and_order_of_favorites(&suggestion_db, &favorites); + builder.extend_list_and_allow_favorites_with_ids(&suggestion_db, 0..4); let list = builder.build(); list.update_filtering("fu"); @@ -446,7 +446,7 @@ pub(crate) mod tests { let logger = Logger::new("test::component_list_modules_tree"); let suggestion_db = mock_suggestion_db(logger); let mut builder = builder::List::new().with_local_scope_module_id(0); - builder.extend(&suggestion_db, 0..11); + builder.extend_list_and_allow_favorites_with_ids(&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 b0461cf139..fcb7c76601 100644 --- a/app/gui/src/controller/searcher/component/builder.rs +++ b/app/gui/src/controller/searcher/component/builder.rs @@ -1,4 +1,21 @@ //! A module with entities used for building proper [`component::List`]. +//! +//! The [`List`] type builds a [`component::List`] with contents sorted as described below: +//! - [`component::Group`]s are sorted alphabetically by name; +//! - [`Component`]s in each [`component::Group`] are ordered: non-modules sorted alphabetically, +//! followed by modules sorted alphabetically; +//! - [`Component`]s and [`component::Group`]s in [`component::List::favorites`] keep the grouping +//! and order set with [`List::set_grouping_and_order_of_favorites`]. +//! +//! When using the methods of the [`List`] type to build a [`component::List`]: +//! - The components and groups are sorted once. +//! - The [`component::List::favorites`] contain only components with IDs that were passed both to +//! [`List::set_grouping_and_order_of_favorites`] and to +//! [`List::extend_list_and_allow_favorites_with_ids`]. +//! - Empty component groups are allowed in favorites. (This simplifies distributing groups of +//! favorites over columns in [Component Browser](crate::controller::Searcher) consistently. +//! That's because for the same input to [`List::set_grouping_and_order_of_favorites`], the same +//! number of groups will be always present in favorites.) use crate::prelude::*; @@ -59,16 +76,16 @@ impl ModuleGroups { // === List === // ============ -/// A [`component::List`] builder. -/// -/// The builder allow extending the list with new entries, and build a list with properly sorted -/// groups. +/// A [`component::List`] builder. See the documentation of the module for more details. #[derive(Clone, Debug, Default)] pub struct List { - all_components: Vec, - module_groups: HashMap, - local_scope: component::Group, - favorites: component::group::List, + all_components: Vec, + module_groups: HashMap, + local_scope: component::Group, + grouping_and_order_of_favorites: component::group::List, + /// IDs of [`Component`]s allowed in [`component::List::favorites`] if they are also present in + /// [`grouping_and_order_of_favorites`]. + allowed_favorites: HashSet, } impl List { @@ -78,9 +95,9 @@ impl List { } /// Return [`List`] with a new [`local_scope`] with its [`Group::component_id`] field set to - /// `module_id`. When the [`extend`] method is called on the returned object, components passed - /// to the method which have their parent module ID equal to `module_id` will be cloned into - /// [`component::List::local_scope`]. + /// `module_id`. When the [`extend_list_and_allow_favorites_with_ids`] method is called on the + /// returned object, components passed to the method which have their parent module ID equal + /// to `module_id` will be cloned into [`component::List::local_scope`]. pub fn with_local_scope_module_id(self, module_id: component::Id) -> Self { const LOCAL_SCOPE_GROUP_NAME: &str = "Local Scope"; let id = Some(module_id); @@ -88,8 +105,10 @@ impl List { Self { local_scope, ..self } } - /// Extend the list with new entries looked up by ID in suggestion database. - pub fn extend( + /// Extend the list with new entries looked up by ID in suggestion database. Allow those + /// entries to be present in [`component::List::favorites`]. See the module documentation for + /// more details. + pub fn extend_list_and_allow_favorites_with_ids( &mut self, db: &model::SuggestionDatabase, entries: impl IntoIterator, @@ -99,6 +118,7 @@ impl List { 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 { + self.allowed_favorites.insert(*component.id); let mut component_inserted_somewhere = false; if let Some(parent_module) = component.suggestion.parent_module() { if let Some(parent_group) = self.lookup_module_group(db, &parent_module) { @@ -124,19 +144,17 @@ impl List { } } - /// Set the favorites in the list. Components are looked up by ID in the suggestion database. - pub fn set_favorites<'a>( + /// Set the grouping and order of [`Components`] in [`component::List::favorites`]. Skips + /// components not present in the suggestion database and skips empty groups. + pub fn set_grouping_and_order_of_favorites<'a>( &mut self, db: &model::SuggestionDatabase, component_groups: impl IntoIterator, ) { - self.favorites = component_groups + self.grouping_and_order_of_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( @@ -164,9 +182,8 @@ impl List { } } - /// Build the list, sorting all group lists and groups' contents appropriately. (Does not sort - /// the [`component::List::favorites`].) - pub fn build(self) -> component::List { + /// Build the list as described in the module's documentation. + pub fn build(mut self) -> component::List { let components_order = component::Order::ByNameNonModulesThenModules; for group in self.module_groups.values() { group.content.update_sorting(components_order); @@ -180,18 +197,29 @@ impl List { top_mdl_bld.extend(top_modules_iter.clone().map(|g| g.content.clone_ref())); 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())); + let favorites = self.build_favorites_and_add_to_all_components(); component::List { - all_components: Rc::new(self.all_components), - top_modules: top_mdl_bld.build(), + all_components: Rc::new(self.all_components), + top_modules: top_mdl_bld.build(), top_modules_flattened: top_mdl_flat_bld.build(), - module_groups: Rc::new( + module_groups: Rc::new( self.module_groups.into_iter().map(|(id, group)| (id, group.build())).collect(), ), - local_scope: self.local_scope, - filtered: default(), - favorites: self.favorites, + local_scope: self.local_scope, + filtered: default(), + favorites, } } + + fn build_favorites_and_add_to_all_components(&mut self) -> component::group::List { + let grouping_and_order = std::mem::take(&mut self.grouping_and_order_of_favorites); + let mut favorites_groups = grouping_and_order.into_iter().collect_vec(); + for group in favorites_groups.iter_mut() { + group.retain_entries(|e| self.allowed_favorites.contains(&e.id)); + self.all_components.extend(group.entries.borrow().iter().cloned()); + } + component::group::List::new(favorites_groups) + } } @@ -231,8 +259,8 @@ mod tests { let mut builder = List::new().with_local_scope_module_id(0); let first_part = (0..3).chain(6..11); let second_part = 3..6; - builder.extend(&suggestion_db, first_part); - builder.extend(&suggestion_db, second_part); + builder.extend_list_and_allow_favorites_with_ids(&suggestion_db, first_part); + builder.extend_list_and_allow_favorites_with_ids(&suggestion_db, second_part); let list = builder.build(); let top_modules: Vec = @@ -326,4 +354,60 @@ mod tests { let expected_ids = vec![5, 6]; assert_eq!(local_scope_ids, expected_ids); } + + /// Test building a component list with non-empty favorites. Verify that the favorites are + /// processed as described in the docs of the [`List::build`] method. + #[test] + fn building_component_list_with_favorites() { + let logger = Logger::new("tests::building_component_list_with_favorites"); + let db = mock_suggestion_db(logger); + let mut builder = List::new(); + let qn_of_db_entry_0 = db.lookup(0).unwrap().qualified_name(); + let qn_of_db_entry_1 = db.lookup(1).unwrap().qualified_name(); + let qn_of_db_entry_3 = db.lookup(3).unwrap().qualified_name(); + const QN_NOT_IN_DB: &str = "test.Test.NameNotInSuggestionDb"; + assert_eq!(db.lookup_by_qualified_name_str(QN_NOT_IN_DB), None); + let groups = [ + execution_context::ComponentGroup { + name: "Group 1".into(), + color: None, + components: vec![ + qn_of_db_entry_0.clone(), + qn_of_db_entry_3.clone(), + QN_NOT_IN_DB.into(), + qn_of_db_entry_3.clone(), + QN_NOT_IN_DB.into(), + qn_of_db_entry_1, + qn_of_db_entry_0, + ], + }, + execution_context::ComponentGroup { + name: "Group 2".into(), + color: None, + components: vec![ + qn_of_db_entry_3.clone(), + QN_NOT_IN_DB.into(), + qn_of_db_entry_3, + QN_NOT_IN_DB.into(), + ], + }, + ]; + builder.set_grouping_and_order_of_favorites(&db, &groups); + builder.extend_list_and_allow_favorites_with_ids(&db, [0, 1, 2].into_iter()); + let list = builder.build(); + let favorites: Vec = list.favorites.iter().map(Into::into).collect(); + let expected = vec![ + ComparableGroupData { + name: "Group 1", + component_id: None, + entries: vec![0, 1, 0], + }, + ComparableGroupData { + name: "Group 2", + component_id: None, + entries: vec![], + }, + ]; + assert_eq!(favorites, expected); + } } diff --git a/app/gui/src/controller/searcher/component/group.rs b/app/gui/src/controller/searcher/component/group.rs index 2b33be0907..b49186609d 100644 --- a/app/gui/src/controller/searcher/component/group.rs +++ b/app/gui/src/controller/searcher/component/group.rs @@ -48,6 +48,12 @@ impl Data { matched_items: Cell::new(0), } } + + fn update_matched_items(&self) { + let entries = self.entries.borrow(); + let matched_items = entries.iter().take_while(|c| !c.is_filtered_out()).count(); + self.matched_items.set(matched_items); + } } @@ -118,6 +124,15 @@ impl Group { }) } + /// Modify the group keeping only the [`Component`]s for which `f` returns [`true`]. + pub fn retain_entries(&mut self, mut f: F) + where F: FnMut(&Component) -> bool { + let group_data = Rc::make_mut(&mut self.data); + group_data.entries.borrow_mut().retain(&mut f); + group_data.initial_entries_order.retain(f); + group_data.update_matched_items(); + } + /// Update the group sorting according to the `order` and update information about matched items /// count. pub fn update_sorting(&self, order: component::Order) { @@ -127,9 +142,7 @@ impl Group { self.sort_by_name_non_modules_then_modules(), component::Order::ByMatch => self.sort_by_match(), } - let entries = self.entries.borrow(); - let matched_items = entries.iter().take_while(|c| !c.is_filtered_out()).count(); - self.matched_items.set(matched_items); + self.update_matched_items(); } fn restore_initial_order(&self) { @@ -230,6 +243,15 @@ impl FromIterator for List { } } +impl IntoIterator for List { + type Item = Group; + type IntoIter = std::vec::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + Rc::unwrap_or_clone(self.groups).into_iter() + } +} + impl Deref for List { type Target = [Group]; fn deref(&self) -> &Self::Target { diff --git a/app/gui/src/lib.rs b/app/gui/src/lib.rs index 948fb1162a..4fc4e4b031 100644 --- a/app/gui/src/lib.rs +++ b/app/gui/src/lib.rs @@ -29,6 +29,7 @@ #![recursion_limit = "512"] // === Features === +#![feature(arc_unwrap_or_clone)] #![feature(async_closure)] #![feature(associated_type_bounds)] #![feature(bool_to_option)]