Skip to content

Commit

Permalink
Implement proper import handling for searcher previews.
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaelMauderer committed Sep 14, 2022
1 parent 64da324 commit 87209e3
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 13 deletions.
67 changes: 59 additions & 8 deletions app/gui/src/controller/searcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,11 @@ impl Searcher {
Self::new_from_graph_controller(parent, ide, project, graph, mode)
}

/// Abort editing and perform cleanup.
pub fn abort_editing(&self) {
self.clear_temporary_imports();
}

/// Create new Searcher Controller, when you have Executed Graph Controller handy.
#[profile(Task)]
pub fn new_from_graph_controller(
Expand Down Expand Up @@ -715,6 +720,7 @@ impl Searcher {
/// Use action at given index as a suggestion. The exact outcome depends on the action's type.
pub fn preview_suggestion(&self, picked_suggestion: action::Suggestion) -> FallibleResult {
tracing::debug!("Previewing suggestion: {:?}", picked_suggestion);
self.clear_temporary_imports();

let id = self.data.borrow().input.next_completion_id();
let picked_completion = FragmentAddedByPickingSuggestion { id, picked_suggestion };
Expand Down Expand Up @@ -748,7 +754,12 @@ impl Searcher {
let intended_method = self.intended_method();
(expression, intended_method)
};
self.add_required_imports(false)?;

let current_fragments = &self.data.borrow().fragments_added_by_picking;
let fragments_added_by_picking =
current_fragments.iter().chain(iter::once(&picked_completion));
self.add_required_imports(fragments_added_by_picking, false)?;

let (expression, intended_method) = expr_and_method();

self.graph.graph().module.with_node_metadata(
Expand Down Expand Up @@ -859,6 +870,7 @@ impl Searcher {
if let Some(guard) = self.node_edit_guard.deref().as_ref() {
guard.prevent_revert()
}
self.clear_temporary_imports();
let expr_and_method = || {
let input_chain = self.data.borrow().input.as_prefix_chain(self.ide.parser());

Expand All @@ -875,7 +887,9 @@ impl Searcher {
let node_id = self.mode.node_id();
// We add the required imports before we edit its content. This way, we avoid an
// intermediate state where imports would already be in use but not yet available.
self.add_required_imports(true)?;
let data_borrowed = self.data.borrow();
let fragments = data_borrowed.fragments_added_by_picking.iter();
self.add_required_imports(fragments, true)?;
let (expression, intended_method) = expr_and_method();
self.graph.graph().set_expression(node_id, expression)?;
if let Mode::NewNode { .. } = self.mode.as_ref() {
Expand Down Expand Up @@ -948,9 +962,13 @@ impl Searcher {
}

#[profile(Debug)]
fn add_required_imports(&self, permanent: bool) -> FallibleResult {
let data_borrowed = self.data.borrow();
let fragments = data_borrowed.fragments_added_by_picking.iter();
fn add_required_imports<'a>(
&self,
fragments: impl Iterator<Item = &'a FragmentAddedByPickingSuggestion>,
permanent: bool,
) -> FallibleResult {
// let data_borrowed = self.data.borrow();
// let fragments = data_borrowed.fragments_added_by_picking.iter();
let imports = fragments.flat_map(|frag| self.code_to_insert(frag).imports);
let mut module = self.module();
let here = self.module_qualified_name();
Expand All @@ -959,20 +977,53 @@ impl Searcher {
let without_enso_project = imports.filter(|i| i.to_string() != ENSO_PROJECT_SPECIAL_MODULE);
for mut import in without_enso_project {
import.remove_main_module_segment();
module.add_module_import(&here, self.ide.parser(), &import);

let import_info = ImportInfo::from_qualified_name(&import);
tracing::warn!("Adding import: {import_info:?}");

let already_exists = module.iter_imports().contains(&import_info);
if already_exists {
continue;
}

module.add_module_import(&here, self.ide.parser(), &import);
self.graph.graph().module.with_import_metadata(
import_info.id(),
Box::new(|import_metadata| {
import_metadata.is_temporary = !permanent;
import_metadata.info = Some(import_info);
}),
)?;
}
self.graph.graph().module.update_ast(module.ast)
}

fn clear_temporary_imports(&self) {
let mut to_remove: Vec<_> = default();
let mut module = self.module();

for import_metadata in self.graph.graph().module.all_import_metadata() {
if import_metadata.is_temporary {
if let Some(import_info) = &import_metadata.info {
if let Err(e) = module.remove_import(import_info) {
tracing::warn!("Failed to remove import because of: {e:?}");
}
to_remove.push(import_info.id());
}
}
}
if let Err(e) = self.graph.graph().module.update_ast(module.ast) {
tracing::warn!("Failed to update module ast when removing imports because of: {e:?}");
}

to_remove.iter().for_each(|id| {
if let Err(e) = self.graph.graph().module.remove_import_metadata(*id) {
tracing::warn!(
"Failed to remove import metadata for import id {id} because of: {e:?}"
);
}
});
}


/// Reload Action List.
///
/// The current list will be set as "Loading" and Language Server will be requested for a new
Expand Down
15 changes: 15 additions & 0 deletions app/gui/src/model/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ pub mod synchronized;

pub use double_representation::module::Id;
use double_representation::module::ImportId;
use double_representation::module::ImportInfo;
pub use double_representation::module::QualifiedName;
pub use double_representation::tp::QualifiedName as TypeQualifiedName;

Expand All @@ -40,6 +41,11 @@ pub use double_representation::tp::QualifiedName as TypeQualifiedName;
#[derive(Debug, Clone, Copy, Fail)]
#[fail(display = "Node with ID {} was not found in metadata.", _0)]
pub struct NodeMetadataNotFound(pub ast::Id);
/// Failure for missing node metadata.
#[derive(Debug, Clone, Copy, Fail)]
#[fail(display = "Import with ID {} was not found in metadata.", _0)]
pub struct ImportMetadataNotFound(pub ImportId);

/// Failed attempt to tread a file path as a module path.
#[derive(Clone, Debug, Fail)]
Expand Down Expand Up @@ -521,6 +527,9 @@ pub struct ImportMetadata {
#[serde(skip_serializing_if = "core::ops::Not::not")]
#[serde(default, deserialize_with = "enso_prelude::deserialize_or_default")]
pub is_temporary: bool,
#[serde(skip_serializing_if = "Option::is_none")]
#[serde(default, deserialize_with = "enso_prelude::deserialize_or_default")]
pub info: Option<ImportInfo>,
}


Expand Down Expand Up @@ -600,6 +609,12 @@ pub trait API: Debug + model::undo_redo::Aware {
fun: Box<dyn FnOnce(&mut ImportMetadata) + '_>,
) -> FallibleResult;

/// Returns the import metadata fof the module.
fn all_import_metadata(&self) -> Vec<ImportMetadata>;

/// Removes the import metadata of the import.
fn remove_import_metadata(&self, id: ImportId) -> FallibleResult<ImportMetadata>;

/// This method exists as a monomorphication for [`with_project_metadata`]. Users are encouraged
/// to use it rather then this method.
///
Expand Down
14 changes: 14 additions & 0 deletions app/gui/src/model/module/plain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::prelude::*;

use crate::model::module::Content;
use crate::model::module::ImportMetadata;
use crate::model::module::ImportMetadataNotFound;
use crate::model::module::Metadata;
use crate::model::module::NodeMetadata;
use crate::model::module::NodeMetadataNotFound;
Expand Down Expand Up @@ -225,6 +226,19 @@ impl model::module::API for Module {
})
}

fn all_import_metadata(&self) -> Vec<ImportMetadata> {
let content = self.content.borrow();
content.metadata.ide.import.values().cloned().collect()
}

fn remove_import_metadata(&self, id: ImportId) -> FallibleResult<ImportMetadata> {
self.try_updating_content(NotificationKind::MetadataChanged, |content| {
let lookup = content.metadata.ide.import.remove(&id);
lookup.ok_or_else(|| ImportMetadataNotFound(id).into())
})
}


fn boxed_with_project_metadata(&self, fun: Box<dyn FnOnce(&ProjectMetadata) + '_>) {
let content = self.content.borrow();
if let Some(metadata) = content.metadata.ide.project.as_ref() {
Expand Down
8 changes: 8 additions & 0 deletions app/gui/src/model/module/synchronized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,14 @@ impl API for Module {
self.model.with_import_metadata(id, fun)
}

fn all_import_metadata(&self) -> Vec<ImportMetadata> {
self.model.all_import_metadata()
}

fn remove_import_metadata(&self, id: ImportId) -> FallibleResult<ImportMetadata> {
self.model.remove_import_metadata(id)
}

fn boxed_with_project_metadata(&self, fun: Box<dyn FnOnce(&ProjectMetadata) + '_>) {
self.model.boxed_with_project_metadata(fun)
}
Expand Down
4 changes: 3 additions & 1 deletion app/gui/src/presenter/searcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,9 @@ impl Searcher {
///
/// This method takes `self`, as the presenter (with the searcher view) should be dropped once
/// editing finishes.
pub fn abort_editing(self) {}
pub fn abort_editing(self) {
self.model.controller.abort_editing()
}

/// Returns the node view that is being edited by the searcher.
pub fn input_view(&self) -> ViewNodeId {
Expand Down
9 changes: 5 additions & 4 deletions app/gui/view/src/searcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,9 @@ impl View {
model.doc_provider.set(docs.clone_ref());
model.list.set_entries(entries);
});
selected_entry <- model.list.selected_entry.on_change();
eval frp.select_action ((id) model.list.select_entry(id));
source.selected_entry <+ model.list.selected_entry;
source.selected_entry <+ selected_entry;
source.size <+ height.value.map(|h| Vector2(SEARCHER_WIDTH,*h));
source.is_visible <+ model.list.size.map(|size| size.x*size.y > std::f32::EPSILON);
// source.is_selected <+ model.documentation.frp.is_selected.map(|&value|value);
Expand All @@ -240,11 +241,11 @@ impl View {
eval frp.show ((()) height.set_target_value(SEARCHER_HEIGHT));
eval frp.hide ((()) height.set_target_value(-list_view::SHADOW_PX));

is_selected <- model.list.selected_entry.map(|e| e.is_some());
is_selected <- selected_entry.map(|e| e.is_some());
is_enabled <- bool(&frp.hide,&frp.show);
is_entry_enabled <- is_selected && is_enabled;
displayed_doc <- model.list.selected_entry.map(f!((id) model.docs_for(*id)));
opt_picked_entry <- model.list.selected_entry.sample(&frp.use_as_suggestion);
displayed_doc <- selected_entry.map(f!((id) model.docs_for(*id)));
opt_picked_entry <- selected_entry.sample(&frp.use_as_suggestion);
source.used_as_suggestion <+ opt_picked_entry.gate(&is_entry_enabled);
source.editing_committed <+ model.list.chosen_entry.gate(&is_entry_enabled);

Expand Down

0 comments on commit 87209e3

Please sign in to comment.