Skip to content

Commit

Permalink
Groups in DocTags (#7337)
Browse files Browse the repository at this point in the history
Fixes #7336 in a quick way.

Next to the old way of defining groups, the library can just add `GROUP` tag to some entities, and it will be added to the group specified in tag's description.

The group name may be qualified (with project name, like `Standard.Base.Input/Output`) or just name - in the latter case, IDE will assume a group defined in the same library as the entity.

Also moved some entities from "export" list in package.yaml to GROUP tag to give an example. I didn't move all of those, as I assume the library team will reorganize those groups anyway.

### Important Notes

@jdunkerley @radeusgd @GregoryTravis When you will start specifying groups in tags, remember that:
* The groups still belongs to a concrete project; if some entity outside a project wants to be added to its group, the "qualified" name should be specified. See `Table.new` example in this PR.
* If the group name does not reflect any group in package.yaml **the tag is ignored**.
* A single entity may be only in a single group. If it's specified in both package.yaml and in tag, the tag takes precedence.

---------

Co-authored-by: Ilya Bogdanov <[email protected]>
  • Loading branch information
farmaazon and vitvakatu authored Jul 24, 2023
1 parent 04ed4c1 commit 1d2371f
Show file tree
Hide file tree
Showing 20 changed files with 194 additions and 79 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,9 @@
- [Help chat][7151]. The link to the Discord server is replaced with a chat
bridge to the Discord server. This is intended to have the chat visible at the
same time as the IDE, so that help can be much more interactive.
- [The libraries' authors may put entities to groups by adding GROUP tag in the
docstring]. It was requested as more convenient way than specifying full names
in package.yaml.

[5910]: https://github.com/enso-org/enso/pull/5910
[6279]: https://github.com/enso-org/enso/pull/6279
Expand All @@ -226,6 +229,7 @@
[7151]: https://github.com/enso-org/enso/pull/7151
[7164]: https://github.com/enso-org/enso/pull/7164
[7372]: https://github.com/enso-org/enso/pull/7372
[7337]: https://github.com/enso-org/enso/pull/7337

#### EnsoGL (rendering engine)

Expand Down
10 changes: 4 additions & 6 deletions app/gui/controller/double-representation/src/name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,9 @@ pub mod project;
pub enum InvalidQualifiedName {
#[fail(display = "The qualified name is empty.")]
EmptyName,
#[fail(display = "No namespace in project qualified name.")]
NoNamespace,
#[fail(display = "Invalid namespace in project qualified name.")]
InvalidNamespace,
#[fail(display = "Too many segments in project qualified name.")]
#[fail(display = "Too few segments in qualified name.")]
TooFewSegments,
#[fail(display = "Too many segments in qualified name.")]
TooManySegments,
}

Expand Down Expand Up @@ -156,7 +154,7 @@ impl QualifiedName {
let mut iter = segments.into_iter().map(|name| name.into());
let project_name = match (iter.next(), iter.next()) {
(Some(ns), Some(name)) => project::QualifiedName::new(ns, name),
_ => return Err(InvalidQualifiedName::NoNamespace.into()),
_ => return Err(InvalidQualifiedName::TooFewSegments.into()),
};
let without_main = iter.skip_while(|s| *s == PROJECTS_MAIN_MODULE);
Ok(Self::new(project_name, without_main.collect()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ impl QualifiedName {
match all_segments.as_slice() {
[namespace, project] => Ok(Self::new(namespace, project)),
[] => Err(InvalidQualifiedName::EmptyName.into()),
[_] => Err(InvalidQualifiedName::NoNamespace.into()),
[_] => Err(InvalidQualifiedName::TooFewSegments.into()),
_ => Err(InvalidQualifiedName::TooManySegments.into()),
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1177,6 +1177,7 @@ pub struct LibraryComponentGroup {
pub color: Option<String>,
pub icon: Option<String>,
/// The list of components provided by this component group.
#[serde(default)]
pub exports: Vec<LibraryComponent>,
}

Expand Down
12 changes: 8 additions & 4 deletions app/gui/src/controller/searcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::prelude::*;

use crate::controller::graph::ImportType;
use crate::controller::graph::RequiredImport;
use crate::model::execution_context::GroupQualifiedName;
use crate::model::module::NodeEditStatus;
use crate::model::suggestion_database;
use crate::presenter::searcher;
Expand Down Expand Up @@ -39,6 +40,8 @@ pub mod input;
/// needed. Currently enabled to trigger engine's caching of user-added nodes.
/// See: https://github.com/enso-org/ide/issues/1067
pub const ASSIGN_NAMES_FOR_NODES: bool = true;
/// A name of a component group containing entries representing literals.
pub const LITERALS_GROUP_NAME: &str = "Literals";


// ==============
Expand Down Expand Up @@ -717,9 +720,9 @@ impl Searcher {

fn add_virtual_entries_to_builder(builder: &mut component::Builder) {
let snippets = component::hardcoded::INPUT_SNIPPETS.with(|s| s.clone());
let group_name = component::hardcoded::INPUT_GROUP_NAME;
let project = project::QualifiedName::standard_base_library();
builder.add_virtual_entries_to_group(group_name, project, snippets);
// Unwrap is safe because conversion from INPUT_GROUP_NAME is tested.
let group_name = GroupQualifiedName::try_from(component::hardcoded::INPUT_GROUP_NAME).unwrap();
builder.add_virtual_entries_to_group(group_name, snippets);
}


Expand Down Expand Up @@ -865,8 +868,9 @@ fn component_list_for_literal(
) -> component::List {
let mut builder = component::builder::Builder::new_empty(db);
let project = project::QualifiedName::standard_base_library();
let group_name = GroupQualifiedName::new(project, LITERALS_GROUP_NAME);
let snippet = component::hardcoded::Snippet::from_literal(literal, db).into();
builder.add_virtual_entries_to_group("Literals", project, vec![snippet]);
builder.add_virtual_entries_to_group(group_name, vec![snippet]);
builder.build()
}

Expand Down
8 changes: 3 additions & 5 deletions app/gui/src/controller/searcher/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ use crate::prelude::*;

use crate::controller::graph::RequiredImport;
use crate::controller::searcher::Filter;
use crate::model::execution_context::GroupQualifiedName;
use crate::model::suggestion_database;

use double_representation::name::project;
use enso_doc_parser::DocSection;
use enso_doc_parser::Tag;
use enso_suggestion_database::entry;
Expand Down Expand Up @@ -47,11 +47,9 @@ const ALIAS_MATCH_ATTENUATION_FACTOR: f32 = 0.75;
#[allow(missing_docs)]
#[derive(Clone, Debug, Default)]
pub struct Group {
/// The project where the group is defined.
pub project: project::QualifiedName,
pub name: ImString,
pub name: GroupQualifiedName,
/// Color as defined in project's `package.yaml` file.
pub color: Option<color::Rgb>,
pub color: Option<color::Rgb>,
}


Expand Down
97 changes: 67 additions & 30 deletions app/gui/src/controller/searcher/component/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ use crate::controller::searcher::component;
use crate::controller::searcher::component::hardcoded;
use crate::controller::searcher::component::Component;
use crate::model::execution_context;
use crate::model::execution_context::GroupQualifiedName;
use crate::model::suggestion_database;

use double_representation::name::project;
use double_representation::name::project::STANDARD_NAMESPACE;
use double_representation::name::QualifiedName;
use double_representation::name::QualifiedNameRef;
Expand Down Expand Up @@ -113,6 +113,7 @@ pub struct Builder<'a> {
built_list: component::List,
/// A mapping from entry id to group index and the cached suggestion database entry.
entry_to_group_map: HashMap<suggestion_database::entry::Id, EntryInGroup>,
group_name_to_id: HashMap<GroupQualifiedName, usize>,
}

impl<'a> Builder<'a> {
Expand All @@ -124,39 +125,45 @@ impl<'a> Builder<'a> {
inside_module: default(),
built_list: default(),
entry_to_group_map: default(),
group_name_to_id: default(),
}
}

/// Create builder for base view (without self type and not inside any module).
pub fn new(db: &'a SuggestionDatabase, groups: &[execution_context::ComponentGroup]) -> Self {
let entry_to_group_entries = groups
.iter()
.enumerate()
.flat_map(|(index, data)| Self::group_data_to_map_entries(db, index, data));
let group_name_to_id =
groups.iter().enumerate().map(|(index, data)| (data.name.clone_ref(), index)).collect();
let entry_to_group_entries = groups.iter().enumerate().flat_map(|(index, data)| {
Self::group_data_to_map_entries(db, index, data, &group_name_to_id)
});
let groups = groups.iter().map(|group_data| component::Group {
project: group_data.project.clone_ref(),
name: group_data.name.clone_ref(),
color: group_data.color,
name: group_data.name.clone_ref(),
color: group_data.color,
});

Self {
db,
this_type: None,
inside_module: None,
built_list: component::List { groups: groups.collect(), ..default() },
entry_to_group_map: entry_to_group_entries.collect(),
group_name_to_id,
}
}

fn group_data_to_map_entries<'b>(
db: &'b SuggestionDatabase,
group_index: usize,
group_data: &'b execution_context::ComponentGroup,
group_name_to_id: &'b HashMap<GroupQualifiedName, usize>,
) -> impl Iterator<Item = (suggestion_database::entry::Id, EntryInGroup)> + 'b {
group_data.components.iter().filter_map(move |component_qn| {
let (id, entry) = db.lookup_by_qualified_name(component_qn).handle_err(|err| {
warn!("Cannot put entry {component_qn} to component groups. {err}")
})?;
Some((id, EntryInGroup { entry, group_index }))
// The group name from documentation tags has precedence.
let group_from_tag = group_index_from_entry_tag(&entry, group_name_to_id);
Some((id, EntryInGroup { entry, group_index: group_from_tag.unwrap_or(group_index) }))
})
}

Expand Down Expand Up @@ -252,17 +259,20 @@ impl<'a> Builder<'a> {
/// It may not be actually added, depending on the mode. See [structure's docs](Builder) for
/// details.
pub fn add_component_from_db(&mut self, id: suggestion_database::entry::Id) -> FallibleResult {
let in_group = self.entry_to_group_map.get(&id);
// If the entry is in group, we already retrieved it from suggestion database.
let entry =
in_group.map_or_else(|| self.db.lookup(id), |group| Ok(group.entry.clone_ref()))?;
// If the entry was specified as in some group, we already retrieved it from suggestion
// database.
let cached_in_group = self.entry_to_group_map.get(&id);
let entry = cached_in_group
.map_or_else(|| self.db.lookup(id), |entry| Ok(entry.entry.clone_ref()))?;
let group_id = cached_in_group
.map(|entry| entry.group_index)
.or_else(|| self.group_index_from_entry_tag(&entry));
let when_displayed = match &self.inside_module {
Some(module) => WhenDisplayed::inside_module(&entry, module.as_ref()),
None if self.this_type.is_some() => WhenDisplayed::with_self_type(),
None => WhenDisplayed::in_base_mode(&entry, in_group.is_some()),
None => WhenDisplayed::in_base_mode(&entry, group_id.is_some()),
};
let component =
Component::new_from_database_entry(id, entry, in_group.map(|e| e.group_index));
let component = Component::new_from_database_entry(id, entry, group_id);
if matches!(when_displayed, WhenDisplayed::Always) {
self.built_list.displayed_by_default.push(component.clone());
}
Expand All @@ -272,19 +282,22 @@ impl<'a> Builder<'a> {
Ok(())
}

fn group_index_from_entry_tag(&self, entry: &suggestion_database::Entry) -> Option<usize> {
group_index_from_entry_tag(entry, &self.group_name_to_id)
}

/// Add virtual entries to a specific group.
///
/// If the groups was not specified in constructor, it will be added.
pub fn add_virtual_entries_to_group(
&mut self,
group_name: &str,
project: project::QualifiedName,
group_name: GroupQualifiedName,
snippets: impl IntoIterator<Item = Rc<hardcoded::Snippet>>,
) {
let groups = &mut self.built_list.groups;
let existing_group_index = groups.iter().position(|grp| grp.name == group_name);
let group_index = existing_group_index.unwrap_or_else(|| {
groups.push(component::Group { project, name: group_name.into(), color: None });
groups.push(component::Group { name: group_name, color: None });
groups.len() - 1
});
for snippet in snippets {
Expand All @@ -299,6 +312,22 @@ impl<'a> Builder<'a> {
}
}

fn group_index_from_entry_tag(
entry: &suggestion_database::Entry,
group_name_to_id: &HashMap<GroupQualifiedName, usize>,
) -> Option<usize> {
entry.group_name.as_ref().and_then(|name| {
// If the group name in tag is not fully qualified, we assume a group defined in the
// same project where entry is defined.
let qn_name =
GroupQualifiedName::try_from(name.as_str()).unwrap_or_else(|_| GroupQualifiedName {
project: entry.defined_in.project().clone_ref(),
name: name.into(),
});
group_name_to_id.get(&qn_name).copied()
})
}



// =============
Expand All @@ -308,27 +337,33 @@ impl<'a> Builder<'a> {
#[cfg(test)]
mod tests {
use super::*;

use crate::controller::searcher::component::tests::check_displayed_components;
use crate::controller::searcher::component::tests::check_groups;

use double_representation::name::project;
use enso_suggestion_database::mock_suggestion_database;
use ide_view::component_browser::component_list_panel::icon;

fn mock_database() -> SuggestionDatabase {
mock_suggestion_database! {
test.Test {
mod TopModule1 {
#[in_group("First Group")]
fn fun2() -> Standard.Base.Any;

mod SubModule1 {
fn fun4() -> Standard.Base.Any;
}
mod SubModule2 {
#[in_group("Second Group")]
fn fun5 -> Standard.Base.Any;
mod SubModule3 {
fn fun6 -> Standard.Base.Any;
}
}

#[in_group("First Group")]
fn fun1() -> Standard.Base.Any;
}
mod TopModule2 {
Expand All @@ -342,20 +377,18 @@ mod tests {
let project = project::QualifiedName::from_text("test.Test").unwrap();
vec![
execution_context::ComponentGroup {
project: project.clone(),
name: "First Group".into(),
name: GroupQualifiedName::new(project.clone_ref(), "First Group"),
color: None,
components: vec![
"test.Test.TopModule2.fun0".try_into().unwrap(),
"test.Test.TopModule1.fun1".try_into().unwrap(),
"test.Test.TopModule1.fun2".try_into().unwrap(),
// Should be overwritten by tag.
"test.Test.TopModule1.SubModule2.fun5".try_into().unwrap(),
],
},
execution_context::ComponentGroup {
project,
name: "Second Group".into(),
color: None,
components: vec!["test.Test.TopModule1.SubModule2.fun5".try_into().unwrap()],
name: GroupQualifiedName::new(project, "Second Group"),
color: None,
components: vec![],
},
]
}
Expand Down Expand Up @@ -444,7 +477,10 @@ mod tests {

// Adding to an empty builder.
let mut builder = Builder::new_empty(&database);
builder.add_virtual_entries_to_group("First Group", project.clone_ref(), snippets.clone());
builder.add_virtual_entries_to_group(
GroupQualifiedName::new(project.clone_ref(), "First Group"),
snippets.clone(),
);
let list = builder.build();
check_displayed_components(&list, vec!["test1", "test2"]);
check_filterable_components(&list, vec!["test1", "test2"]);
Expand All @@ -453,15 +489,16 @@ mod tests {
expected_components: Vec<&str>,
expected_group: Vec<Option<usize>>| {
let mut builder = Builder::new(&database, &groups);
builder.add_virtual_entries_to_group(group_name, project.clone_ref(), snippets.clone());
let group_name = GroupQualifiedName::new(project.clone_ref(), group_name);
builder.add_virtual_entries_to_group(group_name.clone_ref(), snippets.clone());
builder.add_components_from_db(database.keys());
let list = builder.build();
check_displayed_components(&list, expected_components.clone());
check_groups(&list, expected_group.clone());

let mut builder = Builder::new(&database, &groups);
builder.add_components_from_db(database.keys());
builder.add_virtual_entries_to_group(group_name, project.clone_ref(), snippets.clone());
builder.add_virtual_entries_to_group(group_name, snippets.clone());
let list = builder.build();
check_displayed_components(&list, expected_components);
check_groups(&list, expected_group);
Expand Down
5 changes: 4 additions & 1 deletion app/gui/src/controller/searcher/component/hardcoded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use ide_view::component_browser::component_list_panel::grid::entry::icon::Id as

/// Name of the favorites component group in the `Standard.Base` library where virtual components
/// created from the [`INPUT_SNIPPETS`] should be added.
pub const INPUT_GROUP_NAME: &str = "Input";
pub const INPUT_GROUP_NAME: &str = "Standard.Base.Input";
/// Qualified name of the `Text` type.
const TEXT_ENTRY: &str = "Standard.Base.Main.Data.Text.Text";
/// Qualified name of the `Number` type.
Expand Down Expand Up @@ -139,11 +139,14 @@ impl Snippet {
mod tests {
use super::*;

use crate::model::execution_context::GroupQualifiedName;

/// Test that the qualified names used for hardcoded snippets can be constructed. We don't check
/// if the entries are actually available in the suggestion database.
#[test]
fn test_qualified_names_construction() {
QualifiedName::from_text(TEXT_ENTRY).unwrap();
QualifiedName::from_text(NUMBER_ENTRY).unwrap();
GroupQualifiedName::try_from(INPUT_GROUP_NAME).unwrap();
}
}
Loading

0 comments on commit 1d2371f

Please sign in to comment.