enso-org/enso
1
1
Fork 0
mirror of https://github.com/enso-org/enso.git synced 2024-11-26 08:52:58 +03:00
Code Issues Projects Releases Wiki Activity

Create and edit nodes in temporary ast nodes with execution context disabled (#7779)

Browse Source 
Closes #6417

Creating and editing nodes with a component browser does not edit the node in place but creates a temporary AST node with the execution context switch. When the node is committed, the original AST is replaced by the new one, and the temporary node is removed.

This solves several issues:
1. Previewing suggestions or editing nodes should no longer produce unwanted side effects, as the edited node expression is always evaluated with the output context being disabled.
2. Errors no longer propagate down the graph when you edit the node. The edited node has no outputs connected. Inputs behave normally, though.
3. Undo-redo and editing abort is much simpler. To abort editing, remove the temporary node.

https://github.com/enso-org/enso/assets/6566674/8d38e6bc-a4de-4e69-8d48-52d9b45b6554

# Important Notes
The code is a bit awkward in a few places. This is the consequence of previous architecture.
This commit is contained in:
Ilya Bogdanov 2023-10-06 18:56:09 +04:00 committed by GitHub
parent 4d9bc325ef
commit 3b6f13c7b9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 337 additions and 307 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
app/gui/controller/double-representation/src/graph.rs
View File

@ -4,6 +4,7 @@ use crate::prelude::*;
use crate::connection;
use crate::connection::Connection;
use crate::context_switch::ContextSwitch;
use crate::definition;
use crate::definition::DefinitionInfo;
use crate::definition::DefinitionProvider;
@ -14,6 +15,7 @@ use crate::node::NodeInfo;
use ast::known;
use ast::Ast;
use ast::BlockLine;
use engine_protocol::language_server::ExecutionEnvironment;
@ -201,6 +203,28 @@ impl GraphInfo {
})
}
/// Sets expression of the previewed node. Similar to `edit_node`, but also adds the context
/// switch statement to disable the output context. This way execution of the previewed node
/// will not produce unwanted side effects. `execution_environment` is the name of environment
/// output context should be disabled in.
#[profile(Debug)]
pub fn edit_preview_node(
&mut self,
node_id: ast::Id,
new_expression: Ast,
execution_environment: ExecutionEnvironment,
) -> FallibleResult {
self.update_node(node_id, |mut node| {
node.set_expression(new_expression);
node.set_context_switch(crate::context_switch::ContextSwitchExpression {
switch: ContextSwitch::Disable,
context: crate::context_switch::Context::Output,
environment: execution_environment.to_string().into(),
});
Some(node)
})
}
#[cfg(test)]
pub fn expect_code(&self, expected_code: impl Str) {
let code = self.source.ast.repr();

19
app/gui/src/controller/graph.rs
View File

@ -26,6 +26,7 @@ use double_representation::node::NodeAst;
use double_representation::node::NodeInfo;
use double_representation::node::NodeLocation;
use engine_protocol::language_server;
use engine_protocol::language_server::ExecutionEnvironment;
use parser::Parser;
use span_tree::action::Action;
use span_tree::action::Actions;
@ -969,6 +970,24 @@ impl Handle {
})
}
/// Sets the previewed node expression. Similar to `set_expression_ast`, but also adds an
/// execution context switch to the expression to disable the output context of the node.
/// This way the execution of the previewed node will not produce unwanted side effects.
#[profile(Debug)]
pub fn set_preview_expression_ast(
&self,
id: ast::Id,
expression: Ast,
execution_enviroment: ExecutionEnvironment,
) -> FallibleResult {
info!("Setting previewed node {id} expression to `{}`", expression.repr());
self.update_definition_ast(|definition| {
let mut graph = GraphInfo::from_definition(definition);
graph.edit_preview_node(id, expression, execution_enviroment)?;
Ok(graph.source)
})
}
/// Updates the given node's expression by rewriting a part of it, as specified by span crumbs.
///
/// This will not modify AST IDs of any part of the expression that is not selected by the

263
app/gui/src/controller/searcher.rs
View File

@ -7,7 +7,6 @@ use crate::controller::graph::ImportType;
use crate::controller::graph::RequiredImport;
use crate::controller::searcher::breadcrumbs::BreadcrumbEntry;
use crate::model::execution_context::GroupQualifiedName;
use crate::model::module::NodeEditStatus;
use crate::model::suggestion_database;
use crate::presenter::searcher;
@ -153,8 +152,10 @@ pub enum Mode {
/// Searcher is working with a newly created node. `source_node` is either a selected node
/// or a node from which the connection was dragged out before being dropped at the scene.
NewNode { node_id: ast::Id, source_node: Option<ast::Id> },
/// Searcher should edit existing node's expression.
EditNode { node_id: ast::Id },
/// Searcher should edit existing node's expression. `edited_node_id` is the temporary node
/// created to preview the edited expression, while the `original_node_id` is the original node
/// in the graph that gets updated only in case of successful commit.
EditNode { edited_node_id: ast::Id, original_node_id: ast::Id },
}
impl Mode {
@ -162,7 +163,7 @@ impl Mode {
pub fn node_id(&self) -> ast::Id {
match self {
Mode::NewNode { node_id, .. } => *node_id,
Mode::EditNode { node_id, .. } => *node_id,
Mode::EditNode { edited_node_id: node_id, .. } => *node_id,
}
}
@ -265,7 +266,6 @@ pub struct Searcher {
this_arg: Rc<Option<ThisNode>>,
position_in_code: Immutable<Location<Byte>>,
project: model::Project,
node_edit_guard: Rc<Option<EditGuard>>,
}
impl Searcher {
@ -280,12 +280,11 @@ impl Searcher {
position_in_code: Location<Byte>,
) -> FallibleResult<Self> {
let project = project.clone_ref();
let data = if let Mode::EditNode { node_id } = mode {
Data::new_with_edited_node(&graph.graph(), node_id, cursor_position)?
let data = if let Mode::EditNode { original_node_id, .. } = mode {
Data::new_with_edited_node(&graph.graph(), original_node_id, cursor_position)?
} else {
default()
};
let node_metadata_guard = Rc::new(Some(EditGuard::new(&mode, graph.clone_ref())));
let this_arg = Rc::new(match mode {
Mode::NewNode { source_node: Some(node), .. } => ThisNode::new(node, &graph.graph()),
_ => None,
@ -303,7 +302,6 @@ impl Searcher {
language_server: project.json_rpc(),
position_in_code: Immutable(position_in_code),
project,
node_edit_guard: node_metadata_guard,
};
Ok(ret.init())
}
@ -514,12 +512,6 @@ impl Searcher {
/// If `suggestion` is specified, the preview will contains code after applying it.
/// Otherwise it will be just the current searcher input.
pub fn preview(&self, suggestion: Option<&component::Suggestion>) -> FallibleResult {
let transaction_name = "Previewing Component Browser suggestion.";
let _skip = self
.graph
.undo_redo_repository()
.open_ignored_transaction_or_ignore_current(transaction_name);
debug!("Updating node preview. Previewed suggestion: \"{suggestion:?}\".");
self.clear_temporary_imports();
let has_this = self.this_var().is_some();
@ -551,7 +543,12 @@ impl Searcher {
current_input_requirements.chain(picked_suggestion_requirement.iter().cloned());
self.graph.graph().add_required_imports(all_requirements, ImportType::Temporary)?;
}
self.graph.graph().set_expression_ast(self.mode.node_id(), expression)?;
let execution_environment = self.graph.execution_environment();
self.graph.graph().set_preview_expression_ast(
self.mode.node_id(),
expression,
execution_environment,
)?;
Ok(())
}
@ -579,7 +576,12 @@ impl Searcher {
self.graph.graph().add_required_imports(requirements, ImportType::Permanent)?;
}
let node_id = self.mode.node_id();
// In case of editing the node, we select the original node id here, as we want to update
// the original node, not the temporary one.
let node_id = match *self.mode {
Mode::NewNode { node_id, .. } => node_id,
Mode::EditNode { original_node_id, .. } => original_node_id,
};
let graph = self.graph.graph();
graph.set_expression_ast(node_id, self.get_expression(expression.elem))?;
if let Mode::NewNode { .. } = *self.mode {
@ -588,11 +590,6 @@ impl Searcher {
if let Some(this) = self.this_arg.deref().as_ref() {
this.introduce_pattern(graph.clone_ref())?;
}
// Should go last, as we want to prevent a revert only when the committing process was
// successful.
if let Some(guard) = self.node_edit_guard.deref().as_ref() {
guard.prevent_revert()
}
Ok(node_id)
}
@ -612,11 +609,6 @@ impl Searcher {
}
fn clear_temporary_imports(&self) {
let transaction_name = "Clearing temporary imports after closing searcher.";
let _skip = self
.graph
.undo_redo_repository()
.open_ignored_transaction_or_ignore_current(transaction_name);
self.graph.graph().clear_temporary_imports();
}
@ -756,137 +748,6 @@ fn add_virtual_entries_to_builder(builder: &mut component::Builder) {
}
// === Node Edit Metadata Guard ===
/// On creation the `EditGuard` saves the current expression of the node to its metadata.
/// When dropped the metadata is cleared again and, by default, the node content is reverted to the
/// previous expression. The expression reversion can be prevented by calling `prevent_revert`.
///
/// This structure does not create any Undo Redo transactions, but may contribute to existing one
/// if opened somewhere else.
#[derive(Debug)]
struct EditGuard {
node_id: ast::Id,
graph: controller::ExecutedGraph,
revert_expression: Cell<bool>,
}
impl EditGuard {
pub fn new(mode: &Mode, graph: controller::ExecutedGraph) -> Self {
debug!("Initialising EditGuard.");
let ret = Self { node_id: mode.node_id(), graph, revert_expression: Cell::new(true) };
ret.save_node_expression_to_metadata(mode).unwrap_or_else(|e| {
error!("Failed to save the node edit metadata due to error: {}", e)
});
ret
}
pub fn prevent_revert(&self) {
self.revert_expression.set(false);
}
/// Mark the node as edited in its metadata and save the current expression, so it can later
/// be restored.
fn save_node_expression_to_metadata(&self, mode: &Mode) -> FallibleResult {
let transaction_name = "Storing edited node original expression.";
let _skip = self
.graph
.undo_redo_repository()
.open_ignored_transaction_or_ignore_current(transaction_name);
let module = &self.graph.graph().module;
match mode {
Mode::NewNode { .. } => module.with_node_metadata(
self.node_id,
Box::new(|m| {
m.edit_status = Some(NodeEditStatus::Created {});
}),
),
Mode::EditNode { .. } => {
let node = self.graph.graph().node(self.node_id)?;
let previous_expression = node.info.expression().to_string();
module.with_node_metadata(
self.node_id,
Box::new(|metadata| {
metadata.edit_status = Some(NodeEditStatus::Edited { previous_expression });
}),
)
}
}
}
/// Mark the node as no longer edited and discard the edit metadata.
fn clear_node_edit_metadata(&self) -> FallibleResult {
let transaction_name = "Storing edited node original expression.";
let _skip = self
.graph
.undo_redo_repository()
.open_ignored_transaction_or_ignore_current(transaction_name);
let module = &self.graph.graph().module;
module.with_node_metadata(
self.node_id,
Box::new(|metadata| {
metadata.edit_status = None;
}),
)
}
fn get_saved_expression(&self) -> FallibleResult<Option<NodeEditStatus>> {
let module = &self.graph.graph().module;
Ok(module.node_metadata(self.node_id)?.edit_status)
}
fn revert_node_expression_edit(&self) -> FallibleResult {
let transaction_name = "Reverting node expression to original.";
let _skip = self
.graph
.undo_redo_repository()
.open_ignored_transaction_or_ignore_current(transaction_name);
let edit_status = self.get_saved_expression()?;
match edit_status {
None => {
error!(
"Tried to revert the expression of the edited node, \
but found no edit metadata."
);
}
Some(NodeEditStatus::Created) => {
debug!("Deleting temporary node {} after aborting edit.", self.node_id);
self.graph.graph().remove_node(self.node_id)?;
}
Some(NodeEditStatus::Edited { previous_expression }) => {
debug!(
"Reverting expression of node {} to {} after aborting edit.",
self.node_id, &previous_expression
);
let graph = self.graph.graph();
graph.set_expression(self.node_id, previous_expression)?;
}
};
Ok(())
}
}
impl Drop for EditGuard {
fn drop(&mut self) {
if self.revert_expression.get() {
self.revert_node_expression_edit().unwrap_or_else(|e| {
error!("Failed to revert node edit after editing ended because of an error: {e}")
});
} else {
debug!("Not reverting node expression after edit.")
}
if self.graph.graph().node_exists(self.node_id) {
self.clear_node_edit_metadata().unwrap_or_else(|e| {
error!(
"Failed to clear node edit metadata after editing ended because of an error: {e}"
)
});
}
}
}
// === Helpers ===
/// Build a component list with a single component, representing the given literal. When used as a
@ -913,6 +774,7 @@ fn component_list_for_literal(
pub mod test {
use super::*;
use crate::controller::graph::NewNodeInfo;
use crate::controller::ide::plain::ProjectOperationsNotSupported;
use crate::executor::test_utils::TestWithLocalPoolExecutor;
use crate::presenter::searcher::apply_this_argument;
@ -1022,7 +884,6 @@ pub mod test {
ide.expect_manage_projects()
.returning_st(move || Err(ProjectOperationsNotSupported.into()));
ide.expect_are_component_browser_private_entries_visible().returning_st(|| false);
let node_metadata_guard = default();
let breadcrumbs = Breadcrumbs::new();
let searcher = Searcher {
graph,
@ -1036,15 +897,10 @@ pub mod test {
this_arg: Rc::new(this),
position_in_code: Immutable(code.last_line_end_location()),
project: project.clone_ref(),
node_edit_guard: node_metadata_guard,
};
Fixture { data, test, searcher, database }
}
fn new() -> Self {
Self::new_custom(|_| default(), |_, _| {})
}
fn lookup_suggestion(&self, name: &QualifiedName) -> component::Suggestion {
let (id, entry) =
self.database.lookup_by_qualified_name(name).expect("Database lookup failed.");
@ -1492,83 +1348,20 @@ pub mod test {
assert_eq!(module.ast().repr(), expected_code);
// Edit existing node.
searcher.mode = Immutable(Mode::EditNode { node_id: node1.info.id() });
let mut tmp_node_info = NewNodeInfo::new_pushed_back("Nothing");
tmp_node_info.introduce_pattern = false;
let tmp_node = searcher.graph.graph().add_node(tmp_node_info).unwrap();
searcher.mode = Immutable(Mode::EditNode {
edited_node_id: tmp_node,
original_node_id: node1.info.id(),
});
searcher.commit_node().unwrap();
searcher.graph.graph().remove_node(tmp_node).unwrap();
let expected_code =
"import test.Test.Test\nmain =\n Test.test_method\n operator1 = Test.test_method";
assert_eq!(module.ast().repr(), expected_code);
}
#[test]
fn edit_guard() {
let Fixture { test: _test, mut searcher, .. } = Fixture::new();
let graph = searcher.graph.graph();
let node = graph.nodes().unwrap().last().unwrap().clone();
let initial_node_expression = node.expression();
let node_id = node.info.id();
searcher.mode = Immutable(Mode::EditNode { node_id });
searcher.node_edit_guard =
Rc::new(Some(EditGuard::new(&searcher.mode, searcher.graph.clone_ref())));
// Apply an edit to the node.
graph.set_expression(node_id, "Edited Node").unwrap();
// Verify the metadata was initialised after the guard creation.
let module = graph.module.clone_ref();
module
.with_node_metadata(
node_id,
Box::new(|metadata| {
assert_eq!(
metadata.edit_status,
Some(NodeEditStatus::Edited {
previous_expression: node.info.expression().to_string(),
})
);
}),
)
.unwrap();
// Verify the metadata is cleared after the searcher is dropped.
drop(searcher);
module
.with_node_metadata(
node_id,
Box::new(|metadata| {
assert_eq!(metadata.edit_status, None);
}),
)
.unwrap();
// Verify the node was reverted.
let node = graph.nodes().unwrap().last().unwrap().clone();
let final_node_expression = node.expression();
assert_eq!(initial_node_expression.to_string(), final_node_expression.to_string());
}
#[test]
fn edit_guard_no_revert() {
let Fixture { test: _test, mut searcher, .. } = Fixture::new();
let graph = searcher.graph.graph();
let node = graph.nodes().unwrap().last().unwrap().clone();
let node_id = node.info.id();
searcher.mode = Immutable(Mode::EditNode { node_id });
searcher.node_edit_guard =
Rc::new(Some(EditGuard::new(&searcher.mode, searcher.graph.clone_ref())));
// Apply an edit to the node.
let new_expression = "Edited Node";
graph.set_expression(node_id, new_expression).unwrap();
// Prevent reverting the node by calling the `prevent_revert` method.
searcher.node_edit_guard.deref().as_ref().unwrap().prevent_revert();
// Verify the node is not reverted after the searcher is dropped.
drop(searcher);
let node = graph.nodes().unwrap().last().unwrap().clone();
let final_node_expression = node.expression();
assert_eq!(final_node_expression.to_string(), new_expression);
}
/// Test recognition of qualified names in the searcher's input.
#[test]
fn recognize_qualified_names() {

1
app/gui/src/model/undo_redo.rs
View File

@ -322,6 +322,7 @@ impl Repository {
} else if !transaction.ignored.get() {
// If the transaction was not ignored, we will add it to the undo stack.
let frame = transaction.frame.borrow().clone();
debug!("Closing transaction '{transaction}'");
self.new_undo_frame(frame);
} else {
debug!(

41
app/gui/src/presenter/graph.rs
View File

@ -7,6 +7,7 @@ use enso_web::traits::*;
use crate::controller::graph::widget::Request as WidgetRequest;
use crate::controller::upload::NodeFromDroppedFileHandler;
use crate::executor::global::spawn_stream_handler;
use crate::presenter::graph::state::Node;
use crate::presenter::graph::state::State;
use double_representation::context_switch::Context;
@ -688,8 +689,13 @@ impl ViewUpdate {
/// [`presenter::Searcher`] and [`presenter::CallStack`] respectively).
#[derive(Clone, CloneRef, Debug)]
pub struct Graph {
network: frp::Network,
model: Rc<Model>,
network: frp::Network,
model: Rc<Model>,
/// Force the immediate synchronization of the node view with the AST node.
///
/// It is needed in cases when we change the assigned view id of the node, as the regular
/// update from the controllers will not be necessarily triggered.
pub force_view_update: frp::Source<ast::Id>,
}
impl Graph {
@ -702,10 +708,13 @@ impl Graph {
project_view: &view::project::View,
) -> Self {
let network = frp::Network::new("presenter::Graph");
frp::extend! { network
force_view_update <- source();
}
let graph_editor_view = project_view.graph().clone_ref();
let project_view_clone = project_view.clone_ref();
let model = Rc::new(Model::new(project, controller, graph_editor_view, project_view_clone));
Self { network, model }.init(project_view)
Self { network, model, force_view_update }.init(project_view)
}
#[profile(Detail)]
@ -803,6 +812,32 @@ impl Graph {
eval file_upload_requested (((file,position)) model.file_dropped(file.clone_ref(),*position));
}
// Forcefully update the view to match the state of the presenter.
frp::extend! { network
let force_view_update = self.force_view_update.clone_ref();
node_and_ast_id <- force_view_update.filter_map(f!([model](id)
Some((model.state.get_node(*id)?, *id)))
);
displayed_node <- node_and_ast_id._0();
view.set_node_expression <+ displayed_node.map(Node::expression_update).unwrap();
view.set_node_skip <+ displayed_node.map(Node::skip_update).unwrap();
view.set_node_freeze <+ displayed_node.map(Node::freeze_update).unwrap();
view.set_node_context_switch <+ displayed_node.map(Node::output_context_update).unwrap();
view.set_node_position <+ displayed_node.map(Node::position_update).unwrap();
vis_update <- displayed_node.map(Node::visualization_update).unwrap();
enable_vis <- vis_update.filter_map(|(id,path)| path.is_some().as_some(*id));
disable_vis <- vis_update.filter_map(|(id,path)| path.is_none().as_some(*id));
view.enable_visualization <+ enable_vis;
view.disable_visualization <+ disable_vis;
view.set_visualization <+ vis_update;
expression_type <- node_and_ast_id.filter_map(f!([model]((node, ast))
Some((node.view_id?, *ast, model.expression_type(*ast))))
);
view.set_expression_usage_type <+ expression_type;
view.set_node_error_status <+ displayed_node.map(Node::error_update).unwrap();
view.set_node_pending_status <+ displayed_node.map(Node::pending_update).unwrap();
}
view.remove_all_nodes();
update_view.emit(());
self.setup_controller_notification_handlers(update_view, update_expressions);

83
app/gui/src/presenter/graph/state.rs
View File

@ -8,6 +8,7 @@ use crate::presenter::graph::AstNodeId;
use crate::presenter::graph::ViewConnection;
use crate::presenter::graph::ViewNodeId;
use double_representation::context_switch::ContextSwitch;
use double_representation::context_switch::ContextSwitchExpression;
use engine_protocol::language_server::ExpressionUpdatePayload;
use engine_protocol::language_server::SuggestionId;
@ -26,21 +27,59 @@ use ide_view::graph_editor::EdgeEndpoint;
#[allow(missing_docs)]
#[derive(Clone, Debug, Default)]
pub struct Node {
pub view_id: Option<ViewNodeId>,
pub position: Vector2,
pub expression: node_view::Expression,
pub is_skipped: bool,
pub is_frozen: bool,
pub view_id: Option<ViewNodeId>,
pub position: Vector2,
pub expression: node_view::Expression,
pub is_skipped: bool,
pub is_frozen: bool,
pub context_switch: Option<ContextSwitchExpression>,
pub error: Option<node_view::Error>,
pub is_pending: bool,
pub visualization: Option<visualization_view::Path>,
pub error: Option<node_view::Error>,
pub is_pending: bool,
pub visualization: Option<visualization_view::Path>,
/// Indicate whether this node view is updated automatically by changes from the controller
/// or view, or will be explicitly updated..
/// or view, or will be explicitly updated.
disable_expression_auto_update: bool,
}
// === Prepare view updates from the node state ===
#[allow(missing_docs)]
impl Node {
pub fn position_update(&self) -> Option<(ViewNodeId, Vector2)> {
Some((self.view_id?, self.position))
}
pub fn expression_update(&self) -> Option<(ViewNodeId, node_view::Expression)> {
Some((self.view_id?, self.expression.clone()))
}
pub fn skip_update(&self) -> Option<(ViewNodeId, bool)> {
Some((self.view_id?, self.is_skipped))
}
pub fn freeze_update(&self) -> Option<(ViewNodeId, bool)> {
Some((self.view_id?, self.is_frozen))
}
pub fn output_context_update(&self) -> Option<(ViewNodeId, Option<bool>)> {
let switch = self.context_switch.as_ref().map(|expr| expr.switch == ContextSwitch::Enable);
Some((self.view_id?, switch))
}
pub fn error_update(&self) -> Option<(ViewNodeId, Option<node_view::Error>)> {
Some((self.view_id?, self.error.clone()))
}
pub fn pending_update(&self) -> Option<(ViewNodeId, bool)> {
Some((self.view_id?, self.is_pending))
}
pub fn visualization_update(&self) -> Option<(ViewNodeId, Option<visualization_view::Path>)> {
Some((self.view_id?, self.visualization.clone()))
}
}
/// The set of node states.
///
/// This structure allows to access data of any node by Ast ID, or view id. It also keeps list
@ -117,16 +156,23 @@ impl Nodes {
view_id: ViewNodeId,
ast_id: AstNodeId,
) -> &mut Node {
let mut displayed =
Self::get_mut_or_create_static(&mut self.nodes, &mut self.nodes_without_view, ast_id);
if let Some(old_view) = displayed.view_id {
self.ast_node_by_view_id.remove(&old_view);
let mut displayed = self.nodes.entry(ast_id).or_default();
let old_ast_to_remove = if let Some(old_view) = displayed.view_id {
self.ast_node_by_view_id.remove(&old_view)
} else {
self.nodes_without_view.remove_item(&ast_id);
}
None
};
displayed.view_id = Some(view_id);
self.ast_node_by_view_id.insert(view_id, ast_id);
displayed
// That is quite unfortunate, but we have to temporarily drop the acquired borrow over
// `nodes` to remove the old ast. It is needed to ensure we never have two displayed nodes
// updated from a single ast node.
if let Some(old_ast) = old_ast_to_remove {
self.nodes.remove(&old_ast);
}
// Unwrap is safe here, because we just inserted the node to the map.
self.nodes.get_mut(&ast_id).unwrap()
}
/// Update the state retaining given set of nodes. Returns the list of removed nodes' views.
@ -234,6 +280,11 @@ pub struct State {
}
impl State {
/// Get the node state by the AST ID.
pub fn get_node(&self, node: AstNodeId) -> Option<Node> {
self.nodes.borrow().get(node).cloned()
}
/// Get node's view id by the AST ID.
pub fn view_id_of_ast_node(&self, node: AstNodeId) -> Option<ViewNodeId> {
self.nodes.borrow().get(node).and_then(|n| n.view_id)

35
app/gui/src/presenter/searcher.rs
View File

@ -16,7 +16,6 @@ use ide_view::component_browser::component_list_panel::grid as component_grid;
use ide_view::graph_editor::GraphEditor;
use ide_view::graph_editor::NodeId;
use ide_view::project::SearcherParams;
use ide_view::project::SearcherType;
// ==============
@ -47,29 +46,13 @@ pub trait SearcherPresenter: Debug {
let SearcherParams { input, .. } = parameters;
let ast_node = graph_presenter.ast_node_of_view(input);
let (new_node, source_node) =
create_input_node(parameters, graph_presenter, graph_editor, graph_controller)?;
let mode = match ast_node {
Some(node_id) => Mode::EditNode { node_id },
None => {
let (new_node, source_node) =
create_input_node(parameters, graph_presenter, graph_editor, graph_controller)?;
Mode::NewNode { node_id: new_node, source_node }
}
Some(node_id) =>
Mode::EditNode { edited_node_id: new_node, original_node_id: node_id },
None => Mode::NewNode { node_id: new_node, source_node },
};
let target_node = mode.node_id();
// We only want to show the preview of the node if it is a component browser searcher.
if matches!(parameters.searcher_type, SearcherType::ComponentBrowser) {
if let Some(target_node_view) = graph_presenter.view_id_of_ast_node(target_node) {
graph_editor.model.with_node(target_node_view, |node| node.show_preview());
}
} else {
warn!("No view associated with node {:?}.", target_node);
}
// We disable auto-updates for the expression of the node, so we can set the expression
// of the input node without triggering an update of the graph. This is used, for example,
// to show a preview of the item selected in the component browser without changing the
// text the user has typed on the searcher input node.
graph_presenter.allow_expression_auto_updates(target_node, false);
Ok(mode)
}
@ -213,16 +196,10 @@ fn create_input_node(
new_node.metadata = Some(metadata);
new_node.introduce_pattern = false;
let transaction_name = "Add code for created node's visualization preview.";
let _transaction = graph_controller
.undo_redo_repository()
.open_ignored_transaction_or_ignore_current(transaction_name);
let _transaction = graph_controller.get_or_open_transaction(transaction_name);
let created_node = graph_controller.add_node(new_node)?;
graph.assign_node_view_explicitly(input, created_node);
// Display searcher preview. It needs to be done _after_ assigning AST node to view, because
// otherwise Visualization Manager would not know to what node the visualization should be
// attached.
graph_editor.show_node_editing_preview(input);
let source_node = source_node.and_then(|id| graph.ast_node_of_view(id.node));

158
app/gui/src/presenter/searcher/component_browser.rs
View File

@ -6,6 +6,7 @@ use crate::prelude::*;
use crate::controller::searcher::Mode;
use crate::controller::searcher::Notification;
use crate::executor::global::spawn_stream_handler;
use crate::model::undo_redo::Transaction;
use crate::presenter;
use crate::presenter::graph::AstNodeId;
use crate::presenter::graph::ViewNodeId;
@ -45,25 +46,51 @@ pub struct NoSuchComponent(component_grid::EntryId);
// === Model ===
// =============
#[derive(Clone, CloneRef, Debug)]
#[derive(Clone, Debug)]
struct Model {
controller: controller::Searcher,
project: view::project::View,
provider: Rc<RefCell<Option<provider::Component>>>,
graph_controller: controller::Graph,
graph_presenter: presenter::Graph,
project: view::project::View,
provider: Rc<RefCell<Option<provider::Component>>>,
input_view: ViewNodeId,
view: component_browser::View,
view: component_browser::View,
mode: Immutable<Mode>,
transaction: Rc<Transaction>,
visualization_was_enabled: bool,
}
impl Model {
#[profile(Debug)]
#[allow(clippy::too_many_arguments)]
fn new(
controller: controller::Searcher,
graph_controller: &controller::Graph,
graph_presenter: &presenter::Graph,
project: view::project::View,
input_view: ViewNodeId,
view: component_browser::View,
mode: Mode,
transaction: Rc<Transaction>,
visualization_was_enabled: bool,
) -> Self {
let provider = default();
Self { controller, project, view, provider, input_view }
let graph_controller = graph_controller.clone_ref();
let graph_presenter = graph_presenter.clone_ref();
let mode = Immutable(mode);
Self {
controller,
graph_controller,
graph_presenter,
project,
view,
provider,
input_view,
mode,
transaction,
visualization_was_enabled,
}
}
#[profile(Debug)]
@ -80,6 +107,12 @@ impl Model {
}
}
fn update_preview(&self) {
if let Err(error) = self.controller.preview_input() {
error!("Failed to preview searcher preview because of error: {error}.")
}
}
fn suggestion_accepted(
&self,
id: component_grid::EntryId,
@ -94,6 +127,22 @@ impl Model {
}
}
fn abort_editing(&self) {
self.transaction.ignore();
self.controller.abort_editing();
if let Mode::EditNode { original_node_id, .. } = *self.mode {
self.reenable_visualization_if_needed();
self.graph_presenter.assign_node_view_explicitly(self.input_view, original_node_id);
// Force view update so resets to the old expression.
self.graph_presenter.force_view_update.emit(original_node_id);
}
let node_id = self.mode.node_id();
if let Err(err) = self.graph_controller.remove_node(node_id) {
error!("Error while removing a temporary node: {err}.");
}
}
fn breadcrumb_selected(&self, id: BreadcrumbId) {
self.controller.select_breadcrumb(id);
}
@ -118,15 +167,41 @@ impl Model {
}
}
fn reenable_visualization_if_needed(&self) {
if let Mode::EditNode { original_node_id, .. } = *self.mode {
if let Some(node_view) = self.graph_presenter.view_id_of_ast_node(original_node_id) {
self.project.graph().model.with_node(node_view, |node| {
if self.visualization_was_enabled {
node.enable_visualization();
}
});
}
}
}
fn expression_accepted(&self, entry_id: Option<component_grid::EntryId>) -> Option<AstNodeId> {
if let Some(entry_id) = entry_id {
self.suggestion_accepted(entry_id);
}
if !self.controller.is_input_empty() {
self.controller.commit_node().map(Some).unwrap_or_else(|err| {
error!("Error while committing node expression: {err}.");
None
})
match self.controller.commit_node() {
Ok(ast_id) => {
if let Mode::EditNode { original_node_id, edited_node_id } = *self.mode {
self.reenable_visualization_if_needed();
self.graph_presenter
.assign_node_view_explicitly(self.input_view, original_node_id);
if let Err(err) = self.graph_controller.remove_node(edited_node_id) {
error!("Error while removing a temporary node: {err}.");
}
}
Some(ast_id)
}
Err(err) => {
error!("Error while committing node expression: {err}.");
None
}
}
} else {
// if input is empty or contains spaces only, we cannot update the node (there is no
// valid AST to assign). Because it is an expected thing, we also do not report error.
@ -178,13 +253,24 @@ impl SearcherPresenter for ComponentBrowserSearcher {
// We get the position for searcher before initializing the input node, because the
// added node will affect the AST, and the position will become incorrect.
let position_in_code = graph_controller.graph().definition_end_location()?;
let graph = graph_controller.graph();
let transaction = graph.get_or_open_transaction("Open searcher");
let mode = Self::init_input_node(parameters, graph_presenter, view.graph(), &graph)?;
let mut visualization_was_enabled = false;
if let Mode::EditNode { original_node_id, .. } = mode {
if let Some(target_node_view) = graph_presenter.view_id_of_ast_node(original_node_id) {
view.graph().model.with_node(target_node_view, |node| {
visualization_was_enabled = node.visualization_enabled.value();
if visualization_was_enabled {
node.disable_visualization();
}
node.show_preview();
});
}
}
let mode = Self::init_input_node(
parameters,
graph_presenter,
view.graph(),
&graph_controller.graph(),
)?;
let searcher_controller = controller::Searcher::new_from_graph_controller(
ide_controller,
@ -206,7 +292,16 @@ impl SearcherPresenter for ComponentBrowserSearcher {
}
let input = parameters.input;
Ok(Self::new(searcher_controller, view, input))
Ok(Self::new(
searcher_controller,
&graph,
graph_presenter,
view,
input,
mode,
transaction,
visualization_was_enabled,
))
}
fn expression_accepted(
@ -219,7 +314,7 @@ impl SearcherPresenter for ComponentBrowserSearcher {
fn abort_editing(self: Box<Self>) {
self.model.controller.abort_editing()
self.model.abort_editing();
}
fn input_view(&self) -> ViewNodeId {
@ -229,23 +324,39 @@ impl SearcherPresenter for ComponentBrowserSearcher {
impl ComponentBrowserSearcher {
#[profile(Task)]
#[allow(clippy::too_many_arguments)]
fn new(
controller: controller::Searcher,
graph_controller: &controller::Graph,
graph_presenter: &presenter::Graph,
view: view::project::View,
input_view: ViewNodeId,
mode: Mode,
transaction: Rc<Transaction>,
visualization_was_enabled: bool,
) -> Self {
let searcher_view = view.searcher().clone_ref();
let model = Rc::new(Model::new(controller, view, input_view, searcher_view));
let model = Rc::new(Model::new(
controller,
graph_controller,
graph_presenter,
view,
input_view,
searcher_view,
mode,
transaction,
visualization_was_enabled,
));
let network = frp::Network::new("presenter::Searcher");
let graph = &model.project.graph().frp;
let browser = &model.view;
frp::extend! { network
eval model.project.searcher_input_changed ([model]((expr, selections)) {
on_input_changed <- model.project.searcher_input_changed.map(f!([model]((expr, selections)) {
let cursor_position = selections.last().map(|sel| sel.end).unwrap_or_default();
model.input_changed(expr, cursor_position);
});
}));
action_list_changed <- any_mut::<()>();
// When the searcher input is changed, we need to update immediately the list of
@ -264,6 +375,7 @@ impl ComponentBrowserSearcher {
let breadcrumbs = &browser.model().documentation.breadcrumbs;
let documentation = &browser.model().documentation;
frp::extend! { network
init <- source_();
eval_ action_list_changed ([model, grid] {
model.provider.take();
let list = model.controller.components();
@ -286,10 +398,16 @@ impl ComponentBrowserSearcher {
documentation.frp.display_documentation <+ docs;
eval grid.active ((entry) model.on_entry_for_docs_selected(*entry));
no_selection <- any(...);
no_selection <+ init.constant(true);
no_selection <+ grid.active.on_change().map(|e| e.is_none());
eval_ grid.suggestion_accepted([]analytics::remote_log_event("component_browser::suggestion_accepted"));
update_preview <- on_input_changed.gate(&no_selection);
eval_ update_preview(model.update_preview());
eval grid.active((entry) model.suggestion_selected(*entry));
eval grid.module_entered((id) model.module_entered(*id));
}
init.emit(());
let weak_model = Rc::downgrade(&model);
let notifications = model.controller.subscribe();

5
app/gui/src/test.rs
View File

@ -300,7 +300,10 @@ pub mod mock {
let mut executor = TestWithLocalPoolExecutor::set_up();
let data = self.clone();
let searcher_target = executed_graph.graph().nodes().unwrap().last().unwrap().id();
let searcher_mode = controller::searcher::Mode::EditNode { node_id: searcher_target };
let searcher_mode = controller::searcher::Mode::EditNode {
original_node_id: searcher_target,
edited_node_id: default(),
};
let position_in_code = executed_graph.graph().definition_end_location().unwrap();
let searcher = controller::Searcher::new_from_graph_controller(
ide.clone_ref(),

2
app/gui/view/graph-editor/src/component/node.rs
View File

@ -262,6 +262,7 @@ ensogl::define_endpoints_2! {
/// and update the node with new expression tree using `set_expression`.
on_expression_modified (span_tree::Crumbs, ImString),
comment (ImString),
visualization_enabled (bool),
context_switch (bool),
skip (bool),
freeze (bool),
@ -755,6 +756,7 @@ impl Node {
is_enabled <- visualization.view_state.map(|state|{
matches!(state,visualization::ViewState::Enabled { has_error: false })
});
out.visualization_enabled <+ is_enabled;
action_bar.set_action_visibility_state <+ is_enabled;
button_set_to_true <- action_bar.user_action_visibility.on_true();
button_set_to_true_without_error <- button_set_to_true.gate_not(&is_error_set);

11
app/gui/view/graph-editor/src/component/node/input/area.rs
View File

@ -579,8 +579,15 @@ impl Area {
object.map_or(initial_position, |object| {
let pos = object.global_position();
let node_pos = self.model.display_object.global_position();
let size = object.computed_size();
pos.xy() - node_pos.xy() + size * 0.5
// There are some cases when the port position is not yet updated, but the port object
// itself exists. In this case, `pos` will contain a zero vector, and we want to use
// `initial_position` instead.
if pos == Vector3::default() {
initial_position
} else {
let size = object.computed_size();
pos.xy() - node_pos.xy() + size * 0.5
}
})
}

2
app/gui/view/graph-editor/src/component/node/input/widget.rs
View File

@ -793,7 +793,7 @@ impl Tree {
}
/// Get pretty-printed representation of this widget tree for debugging purposes.
fn debug_print(&self) -> String {
pub fn debug_print(&self) -> String {
let mut result = String::new();
let hierarchy = self.model.hierarchy.borrow();
let Some(root) = hierarchy.first() else { return "<EMPTY>".to_string() };