Skip to content

Commit

Permalink
Local Scope entries in new Hierarchical Action List. (#3526)
Browse files Browse the repository at this point in the history
Make the local scope components available to the Component Browser through `component::List` (previously known as "Hierarchical Action List"). (See the [Local Scope Section in the Design Doc](https://github.com/enso-org/design/blob/main/epics/component-browser/design.md#local-scope-section) for more details.)

https://www.pivotaltracker.com/story/show/181869186

# Important Notes
- Note: the PR description does not include a screencast due to the changes in this PR not having any visual effect at this moment. The result of this PR's changes would be only observed in the Component Browser, but the Component Browser is not merged yet. As described in the task's Acceptance Criteria, the changes in this PR are currently only covered by tests.
- The `component::group::Data::new_empty_visible` constructor was renamed to `from_name_and_id` and changed to set the `visible` field to a default value. All known code paths appear to call the `update_sorting_and_visibility` method before checking the value of the `visible` field, so setting the value to `true` when creating the object does not seem needed.

[ci no changelog needed]
  • Loading branch information
akavel authored Jun 29, 2022
1 parent 9d76ba9 commit d689777
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 37 deletions.
10 changes: 10 additions & 0 deletions app/gui/src/controller/graph/executed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use crate::model::execution_context::Visualization;
use crate::model::execution_context::VisualizationId;
use crate::model::execution_context::VisualizationUpdateData;

use double_representation::module;
use engine_protocol::language_server::MethodPointer;
use span_tree::generate::context::CalledMethodInfo;
use span_tree::generate::context::Context;
Expand Down Expand Up @@ -301,6 +302,15 @@ impl Handle {
self.graph.borrow().clone_ref()
}

/// Get a full qualified name of the module in the [`graph`]. The name is obtained from the
/// module's path and the `project` name.
pub fn module_qualified_name(
&self,
project: &dyn model::project::API,
) -> module::QualifiedName {
self.graph().module.path().qualified_module_name(project.qualified_name())
}

/// Returns information about all the connections between graph's nodes.
///
/// In contrast with the `controller::Graph::connections` this uses information received from
Expand Down
16 changes: 12 additions & 4 deletions app/gui/src/controller/searcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,9 @@ impl Searcher {
_ => None,
});
let favorites = graph.component_groups();
let list_builder_with_favs = component_list_builder_with_favorites(&database, &*favorites);
let module_name = graph.module_qualified_name(&*project);
let list_builder_with_favs =
component_list_builder_with_favorites(&database, &module_name, &*favorites);
let ret = Self {
logger,
graph,
Expand Down Expand Up @@ -1113,7 +1115,7 @@ impl Searcher {
}

fn module_qualified_name(&self) -> QualifiedName {
self.graph.graph().module.path().qualified_module_name(self.project.qualified_name())
self.graph.module_qualified_name(&*self.project)
}

/// Get the user action basing of current input (see `UserAction` docs).
Expand Down Expand Up @@ -1169,9 +1171,13 @@ impl Searcher {

fn component_list_builder_with_favorites<'a>(
suggestion_db: &model::SuggestionDatabase,
local_scope_module: &QualifiedName,
groups: impl IntoIterator<Item = &'a model::execution_context::ComponentGroup>,
) -> component::builder::List {
let mut builder = component::builder::List::new();
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
}
Expand Down Expand Up @@ -1360,7 +1366,9 @@ pub mod test {
ide.expect_manage_projects()
.returning_st(move || Err(ProjectOperationsNotSupported.into()));
let favorites = graph.component_groups();
let list_bldr_with_favs = component_list_builder_with_favorites(&database, &*favorites);
let module_qn = graph.module_qualified_name(&*project);
let list_builder_with_favs =
component_list_builder_with_favorites(&database, &module_qn, &*favorites);
let searcher = Searcher {
graph,
logger,
Expand All @@ -1373,7 +1381,7 @@ 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),
list_builder_with_favorites: Rc::new(list_builder_with_favs),
};
let entry1 = searcher.database.lookup(1).unwrap();
let entry2 = searcher.database.lookup(2).unwrap();
Expand Down
51 changes: 46 additions & 5 deletions app/gui/src/controller/searcher/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,36 @@ pub type Id = suggestion_database::entry::Id;
pub type MatchInfo = controller::searcher::action::MatchInfo;



// =============
// === Order ===
// =============

/// Defines supported sorting orders for [`Component`]s. Used by
/// [`Group::update_sorting_and_visibility`].
#[derive(Copy, Clone, Debug)]
pub enum Order {
/// Order non-modules by name, followed by modules (also by name).
ByNameNonModulesThenModules,
/// Order [`Component`]s by [`Component::match_info`] score (best scores first).
ByMatch,
}

impl Order {
/// Compare two [`Component`]s according to [`Order`].
fn compare(&self, a: &Component, b: &Component) -> std::cmp::Ordering {
match self {
Order::ByNameNonModulesThenModules => {
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()))
}
Order::ByMatch => a.match_info.borrow().cmp(&*b.match_info.borrow()).reverse(),
}
}
}



// =================
// === Component ===
// =================
Expand Down Expand Up @@ -128,6 +158,9 @@ pub struct List {
top_modules_flattened: group::AlphabeticalList,
module_groups: Rc<HashMap<Id, ModuleGroups>>,
filtered: Rc<Cell<bool>>,
/// Components to display in the "Local Scope" section of the [Component
/// Browser](crate::controller::Searcher).
pub local_scope: Group,
/// Groups of components to display in the "Favorites Data Science Tools" section of the
/// [Component Browser](crate::controller::Searcher).
pub favorites: group::List,
Expand Down Expand Up @@ -163,20 +196,23 @@ impl List {
for component in &*self.all_components {
component.update_matching_info(pattern)
}
let pattern_not_empty = !pattern.is_empty();
let components_order =
if pattern_not_empty { Order::ByMatch } else { Order::ByNameNonModulesThenModules };
for group in self.all_groups_not_in_favorites() {
group.update_sorting_and_visibility(pattern);
group.update_sorting_and_visibility(components_order);
}
for group in self.favorites.iter() {
group.update_visibility();
}
self.filtered.set(!pattern.is_empty());
self.filtered.set(pattern_not_empty);
}

/// All groups from [`List`] without the groups found in [`List::favorites`].
fn all_groups_not_in_favorites(&self) -> impl Iterator<Item = &Group> {
let normal = self.module_groups.values().map(|mg| &mg.content);
let flattened = self.top_modules_flattened.iter();
normal.chain(flattened)
normal.chain(flattened).chain(std::iter::once(&self.local_scope))
}
}

Expand Down Expand Up @@ -301,30 +337,35 @@ pub(crate) mod tests {
suggestion_db.put_entry(id, entry.clone())
}
let favorites = mock_favorites(&suggestion_db, &[3, 2]);
let mut builder = builder::List::new();
let mut builder = builder::List::new().with_local_scope_module_id(0);
builder.set_favorites(&suggestion_db, &favorites);
builder.extend(&suggestion_db, 0..4);
let list = builder.build();

list.update_filtering("fu");
assert_ids_of_matches_entries(&list.top_modules()[0], &[2, 3]);
assert_ids_of_matches_entries(&list.favorites[0], &[3, 2]);
assert_ids_of_matches_entries(&list.local_scope, &[2]);

list.update_filtering("x");
assert_ids_of_matches_entries(&list.top_modules()[0], &[3]);
assert_ids_of_matches_entries(&list.favorites[0], &[3]);
assert_ids_of_matches_entries(&list.local_scope, &[]);

list.update_filtering("Sub");
assert_ids_of_matches_entries(&list.top_modules()[0], &[1]);
assert_ids_of_matches_entries(&list.favorites[0], &[]);
assert_ids_of_matches_entries(&list.local_scope, &[]);

list.update_filtering("y");
assert_ids_of_matches_entries(&list.top_modules()[0], &[]);
assert_ids_of_matches_entries(&list.favorites[0], &[]);
assert_ids_of_matches_entries(&list.local_scope, &[]);

list.update_filtering("");
assert_ids_of_matches_entries(&list.top_modules()[0], &[2, 1]);
assert_ids_of_matches_entries(&list.favorites[0], &[3, 2]);
assert_ids_of_matches_entries(&list.local_scope, &[2]);
}


Expand All @@ -335,7 +376,7 @@ 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();
let mut builder = builder::List::new().with_local_scope_module_id(0);
builder.extend(&suggestion_db, 0..11);
let list = builder.build();

Expand Down
36 changes: 32 additions & 4 deletions app/gui/src/controller/searcher/component/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ impl ModuleGroups {
pub struct List {
all_components: Vec<Component>,
module_groups: HashMap<component::Id, ModuleGroups>,
local_scope: component::Group,
favorites: component::group::List,
}

Expand All @@ -76,12 +77,25 @@ impl List {
default()
}

/// 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`].
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);
let local_scope = component::Group::from_name_and_id(LOCAL_SCOPE_GROUP_NAME, id);
Self { local_scope, ..self }
}

/// Extend the list with new entries looked up by ID in suggestion database.
pub fn extend(
&mut self,
db: &model::SuggestionDatabase,
entries: impl IntoIterator<Item = component::Id>,
) {
use suggestion_database::entry::Kind;
let local_scope_id = self.local_scope.component_id;
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 {
Expand All @@ -90,6 +104,12 @@ impl List {
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;
let parent_id = parent_group.content.component_id;
let in_local_scope = parent_id == local_scope_id && local_scope_id.is_some();
let not_module = component.suggestion.kind != Kind::Module;
if in_local_scope && not_module {
self.local_scope.entries.borrow_mut().push(component.clone_ref());
}
}
if let Some(top_group) = self.lookup_module_group(db, &parent_module.top_module()) {
if let Some(flatten_group) = &mut top_group.flattened_content {
Expand Down Expand Up @@ -147,12 +167,14 @@ 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 {
let components_order = component::Order::ByNameNonModulesThenModules;
for group in self.module_groups.values() {
group.content.update_sorting_and_visibility("");
group.content.update_sorting_and_visibility(components_order);
if let Some(flattened) = &group.flattened_content {
flattened.update_sorting_and_visibility("");
flattened.update_sorting_and_visibility(components_order);
}
}
self.local_scope.update_sorting_and_visibility(components_order);
let top_modules_iter = self.module_groups.values().filter(|g| g.is_top_module);
let mut top_mdl_bld = component::group::AlphabeticalListBuilder::default();
top_mdl_bld.extend(top_modules_iter.clone().map(|g| g.content.clone_ref()));
Expand All @@ -165,6 +187,7 @@ impl List {
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,
}
Expand Down Expand Up @@ -204,8 +227,8 @@ mod tests {
#[test]
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();
let suggestion_db = mock_suggestion_db(logger);
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);
Expand Down Expand Up @@ -297,5 +320,10 @@ mod tests {
.into_iter()
.collect();
assert_eq!(module_subgroups, expected);

let local_scope_entries = &list.local_scope.entries;
let local_scope_ids = local_scope_entries.borrow().iter().map(|e| *e.id).collect_vec();
let expected_ids = vec![5, 6];
assert_eq!(local_scope_ids, expected_ids);
}
}
42 changes: 18 additions & 24 deletions app/gui/src/controller/searcher/component/group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use ensogl::data::color;

/// The [`Group`] fields, which are shared and available by [`AsRef`] and [`Deref`].
#[allow(missing_docs)]
#[derive(Clone, Debug)]
#[derive(Clone, Debug, Default)]
pub struct Data {
pub name: ImString,
pub color: Option<color::Rgb>,
Expand All @@ -32,13 +32,13 @@ pub struct Data {
}

impl Data {
fn new_empty_visible(name: impl Into<ImString>, component_id: Option<component::Id>) -> Self {
fn from_name_and_id(name: impl Into<ImString>, component_id: Option<component::Id>) -> Self {
Data {
name: name.into(),
color: None,
component_id,
entries: default(),
visible: Cell::new(true),
visible: default(),
}
}
}
Expand All @@ -50,7 +50,7 @@ impl Data {
// =============

/// A group of [`Component`]s.
#[derive(Clone, CloneRef, Debug)]
#[derive(Clone, CloneRef, Debug, Default)]
pub struct Group {
data: Rc<Data>,
}
Expand All @@ -63,14 +63,22 @@ impl Deref for Group {
}

impl Group {
/// Create a named empty group referring to module with specified component ID.
pub fn from_name_and_id(
name: impl Into<ImString>,
component_id: Option<component::Id>,
) -> Self {
Self { data: Rc::new(Data::from_name_and_id(name, component_id)) }
}

/// Create empty group referring to some module component.
pub fn from_entry(component_id: component::Id, entry: &suggestion_database::Entry) -> Self {
let name: String = if entry.module.is_top_module() {
(&entry.module).into()
} else {
entry.module.name().into()
};
Self { data: Rc::new(Data::new_empty_visible(name, Some(component_id))) }
Self::from_name_and_id(name, Some(component_id))
}

/// Construct from [`execution_context::ComponentGroup`] components looked up in the suggestion
Expand Down Expand Up @@ -100,25 +108,11 @@ impl Group {
})
}

/// Update the group sorting according to the current filtering pattern and call
/// [`update_visibility`].
pub fn update_sorting_and_visibility(&self, pattern: impl AsRef<str>) {
{
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());
}
}
/// Update the group sorting according to the `order` and call [`update_visibility`].
pub fn update_sorting_and_visibility(&self, order: component::Order) {
// 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.
self.entries.borrow_mut().sort_by(|a, b| order.compare(a, b));
self.update_visibility();
}

Expand Down

0 comments on commit d689777

Please # to comment.