diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ae811e139d..0c6d18d6806 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,9 @@ - [Proper Polyglot Vector and Array Support][3667] - [IDE uses new visualization API.][3661] - [Visualization of long textual values improved][3665] +- [Selecting a suggestion from the searcher or component browser now updates the + visualisation of the edited node to preview the results of applying the + suggestion.][3691] #### EnsoGL (rendering engine) @@ -311,6 +314,7 @@ [3647]: https://github.com/enso-org/enso/pull/3647 [3673]: https://github.com/enso-org/enso/pull/3673 [3684]: https://github.com/enso-org/enso/pull/3684 +[3691]: https://github.com/enso-org/enso/pull/3691 [3695]: https://github.com/enso-org/enso/pull/3695 #### Enso Compiler diff --git a/app/gui/src/controller/searcher.rs b/app/gui/src/controller/searcher.rs index 8fb938bb188..89401bc9f44 100644 --- a/app/gui/src/controller/searcher.rs +++ b/app/gui/src/controller/searcher.rs @@ -49,6 +49,7 @@ pub const ASSIGN_NAMES_FOR_NODES: bool = true; const ENSO_PROJECT_SPECIAL_MODULE: &str = concatcp!(project::STANDARD_BASE_LIBRARY_PATH, ".Enso_Project"); +const MINIMUM_PATTERN_OFFSET: usize = 1; // ============== @@ -606,7 +607,7 @@ impl Searcher { /// in a new action list (the appropriate notification will be emitted). #[profile(Debug)] pub fn set_input(&self, new_input: String) -> FallibleResult { - tracing::debug!("Manually setting input to {}.", new_input); + tracing::debug!("Manually setting input to {new_input}."); let parsed_input = ParsedInput::new(new_input, self.ide.parser())?; let old_expr = self.data.borrow().input.expression.repr(); let new_expr = parsed_input.expression.repr(); @@ -648,29 +649,16 @@ impl Searcher { /// searcher's input will be updated and returned by this function. #[profile(Debug)] pub fn use_suggestion(&self, picked_suggestion: action::Suggestion) -> FallibleResult { - tracing::info!("Picking suggestion: {:?}", picked_suggestion); + tracing::info!("Picking suggestion: {picked_suggestion:?}."); let id = self.data.borrow().input.next_completion_id(); let picked_completion = FragmentAddedByPickingSuggestion { id, picked_suggestion }; let code_to_insert = self.code_to_insert(&picked_completion).code; - tracing::debug!("Code to insert: \"{}\"", code_to_insert); + tracing::debug!("Code to insert: \"{code_to_insert}\""); let added_ast = self.ide.parser().parse_line_ast(&code_to_insert)?; let pattern_offset = self.data.borrow().input.pattern_offset; - let new_expression = match self.data.borrow_mut().input.expression.take() { - None => { - let ast = ast::prefix::Chain::from_ast_non_strict(&added_ast); - ast::Shifted::new(pattern_offset, ast) - } - Some(mut expression) => { - let new_argument = ast::prefix::Argument { - sast: ast::Shifted::new(pattern_offset.max(1), added_ast), - prefix_id: default(), - }; - expression.args.push(new_argument); - expression - } - }; + let new_expression_chain = self.create_new_expression_chain(added_ast, pattern_offset); let new_parsed_input = ParsedInput { - expression: Some(new_expression), + expression: Some(new_expression_chain), pattern_offset: 1, pattern: "".to_string(), }; @@ -681,6 +669,30 @@ impl Searcher { Ok(new_input) } + fn create_new_expression_chain( + &self, + added_ast: Ast, + pattern_offset: usize, + ) -> ast::Shifted { + match self.data.borrow_mut().input.expression.take() { + None => { + let ast = ast::prefix::Chain::from_ast_non_strict(&added_ast); + ast::Shifted::new(pattern_offset, ast) + } + Some(mut expression) => { + let new_argument = ast::prefix::Argument { + sast: ast::Shifted::new( + pattern_offset.max(MINIMUM_PATTERN_OFFSET), + added_ast, + ), + prefix_id: default(), + }; + expression.args.push(new_argument); + expression + } + } + } + /// Use action at given index as a suggestion. The exact outcome depends on the action's type. pub fn use_as_suggestion(&self, index: usize) -> FallibleResult { let error = || NoSuchAction { index }; @@ -696,15 +708,43 @@ impl Searcher { } /// Preview the suggestion in the searcher. - pub fn preview_entry_as_suggestion(&self, index: usize) { - tracing::debug!("Previewing entry: {:?}", index); - //TODO[MM] the actual functionality here will be implemented as part of task #182634050. + pub fn preview_entry_as_suggestion(&self, index: usize) -> FallibleResult { + tracing::debug!("Previewing entry: {index:?}."); + let error = || NoSuchAction { index }; + let suggestion = { + let data = self.data.borrow(); + let list = data.actions.list().ok_or_else(error)?; + list.get_cloned(index).ok_or_else(error)?.action + }; + if let Action::Suggestion(picked_suggestion) = suggestion { + self.preview_suggestion(picked_suggestion)?; + }; + + Ok(()) } /// Use action at given index as a suggestion. The exact outcome depends on the action's type. - pub fn preview_suggestion(&self, selected_suggestion: action::Suggestion) { - //TODO[MM] the actual functionality here will be implemented as part of task #182634050. - tracing::debug!("Previewing suggestion: {:?}", selected_suggestion); + pub fn preview_suggestion(&self, picked_suggestion: action::Suggestion) -> FallibleResult { + tracing::debug!("Previewing suggestion: \"{picked_suggestion:?}\"."); + + let id = self.data.borrow().input.next_completion_id(); + let picked_completion = FragmentAddedByPickingSuggestion { id, picked_suggestion }; + let code_to_insert = self.code_to_insert(&picked_completion).code; + tracing::debug!("Code to insert: \"{code_to_insert}\".",); + let added_ast = self.ide.parser().parse_line_ast(&code_to_insert)?; + let pattern_offset = self.data.borrow().input.pattern_offset; + let new_expression_chain = self.create_new_expression_chain(added_ast, pattern_offset); + let expression = self.get_expression(Some(new_expression_chain)); + let intended_method = self.intended_method(); + + self.graph.graph().module.with_node_metadata( + self.mode.node_id(), + Box::new(|md| md.intended_method = intended_method), + )?; + tracing::debug!("Previewing expression: \"{:?}\".", expression); + self.graph.graph().set_expression(self.mode.node_id(), expression)?; + + Ok(()) } /// Execute given action. @@ -775,7 +815,7 @@ impl Searcher { let list = data.actions.list().ok_or_else(error)?; list.get_cloned(index).ok_or_else(error)?.action }; - tracing::debug!("Previewing action: {:?}", action); + tracing::debug!("Previewing action: {action:?}"); Ok(()) } @@ -804,24 +844,15 @@ impl Searcher { if let Some(guard) = self.node_edit_guard.deref().as_ref() { guard.prevent_revert() } - let expr_and_method = || { - let input_chain = self.data.borrow().input.as_prefix_chain(self.ide.parser()); - let expression = match (self.this_var(), input_chain) { - (Some(this_var), Some(input)) => - apply_this_argument(this_var, &input.wrapped.into_ast()).repr(), - (None, Some(input)) => input.wrapped.into_ast().repr(), - (_, None) => "".to_owned(), - }; - let intended_method = self.intended_method(); - (expression, intended_method) - }; - - 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()?; - let (expression, intended_method) = expr_and_method(); + + let node_id = self.mode.node_id(); + let input_chain = self.data.borrow().input.as_prefix_chain(self.ide.parser()); + let expression = self.get_expression(input_chain); + let intended_method = self.intended_method(); self.graph.graph().set_expression(node_id, expression)?; if let Mode::NewNode { .. } = self.mode.as_ref() { self.graph.graph().introduce_name_on(node_id)?; @@ -837,6 +868,16 @@ impl Searcher { Ok(node_id) } + fn get_expression(&self, input_chain: Option>) -> String { + let expression = match (self.this_var(), input_chain) { + (Some(this_var), Some(input)) => + apply_this_argument(this_var, &input.wrapped.into_ast()).repr(), + (None, Some(input)) => input.wrapped.into_ast().repr(), + (_, None) => "".to_owned(), + }; + expression + } + /// Adds an example to the graph. /// /// The example piece of code will be inserted as a new function definition, and in current @@ -1268,7 +1309,11 @@ impl EditGuard { module.with_node_metadata( self.node_id, Box::new(|metadata| { - metadata.edit_status = Some(NodeEditStatus::Edited { previous_expression }); + let previous_intended_method = metadata.intended_method.clone(); + metadata.edit_status = Some(NodeEditStatus::Edited { + previous_expression, + previous_intended_method, + }); }), ) } @@ -1311,7 +1356,7 @@ impl EditGuard { tracing::debug!("Deleting temporary node {} after aborting edit.", self.node_id); self.graph.graph().remove_node(self.node_id)?; } - Some(NodeEditStatus::Edited { previous_expression }) => { + Some(NodeEditStatus::Edited { previous_expression, previous_intended_method }) => { tracing::debug!( "Reverting expression of node {} to {} after aborting edit.", self.node_id, @@ -1319,6 +1364,13 @@ impl EditGuard { ); let graph = self.graph.graph(); graph.set_expression(self.node_id, previous_expression)?; + let module = &self.graph.graph().module; + module.with_node_metadata( + self.node_id, + Box::new(|metadata| { + metadata.intended_method = previous_intended_method; + }), + )?; } }; Ok(()) @@ -1330,8 +1382,7 @@ impl Drop for EditGuard { if self.revert_expression.get() { self.revert_node_expression_edit().unwrap_or_else(|e| { tracing::error!( - "Failed to revert node edit after editing ended because of an error: {}", - e + "Failed to revert node edit after editing ended because of an error: {e}" ) }); } else { @@ -1340,8 +1391,7 @@ impl Drop for EditGuard { if self.graph.graph().node_exists(self.node_id) { self.clear_node_edit_metadata().unwrap_or_else(|e| { tracing::error!( - "Failed to clear node edit metadata after editing ended because of an error: {}", - e + "Failed to clear node edit metadata after editing ended because of an error: {e}" ) }); } @@ -2412,7 +2462,8 @@ pub mod test { assert_eq!( metadata.edit_status, Some(NodeEditStatus::Edited { - previous_expression: node.info.expression().to_string(), + previous_expression: node.info.expression().to_string(), + previous_intended_method: None, }) ); }), diff --git a/app/gui/src/model/module.rs b/app/gui/src/model/module.rs index 7bc635fd9fd..3c561043d01 100644 --- a/app/gui/src/model/module.rs +++ b/app/gui/src/model/module.rs @@ -369,12 +369,15 @@ pub struct IdeMetadata { } /// Metadata about a nodes edit status. +#[allow(clippy::large_enum_variant)] #[derive(Clone, Debug, Deserialize, PartialEq, Serialize, Eq)] pub enum NodeEditStatus { /// The node was edited and had a previous expression. Edited { /// Expression of the node before the edit was started. - previous_expression: String, + previous_expression: String, + /// Intended method of the node before editing (if known). + previous_intended_method: Option, }, /// The node was created and did not previously exist. Created, diff --git a/app/gui/src/presenter/graph.rs b/app/gui/src/presenter/graph.rs index 0d81baada53..136ea748444 100644 --- a/app/gui/src/presenter/graph.rs +++ b/app/gui/src/presenter/graph.rs @@ -115,7 +115,7 @@ impl Model { /// Node position was changed in view. fn node_position_changed(&self, id: ViewNodeId, position: Vector2) { - self.update_ast( + self.log_action( || { let ast_id = self.state.update_from_view().set_node_position(id, position)?; Some(self.controller.graph().set_node_position(ast_id, position)) @@ -125,7 +125,7 @@ impl Model { } fn node_visualization_changed(&self, id: ViewNodeId, path: Option) { - self.update_ast( + self.log_action( || { let ast_id = self.state.update_from_view().set_node_visualization(id, path.clone())?; @@ -141,9 +141,15 @@ impl Model { ); } + /// Node expression was edited in the view. Should be called whenever the user changes the + /// contents of a node during editing. + fn node_expression_set(&self, id: ViewNodeId, expression: String) { + self.state.update_from_view().set_node_expression(id, expression); + } + /// Node was removed in view. fn node_removed(&self, id: ViewNodeId) { - self.update_ast( + self.log_action( || { let ast_id = self.state.update_from_view().remove_node(id)?; Some(self.controller.graph().remove_node(ast_id)) @@ -154,7 +160,7 @@ impl Model { /// Connection was created in view. fn new_connection_created(&self, id: ViewConnection) { - self.update_ast( + self.log_action( || { let connection = self.view.model.edges.get_cloned_ref(&id)?; let ast_to_create = self.state.update_from_view().create_connection(connection)?; @@ -166,7 +172,7 @@ impl Model { /// Connection was removed in view. fn connection_removed(&self, id: ViewConnection) { - self.update_ast( + self.log_action( || { let ast_to_remove = self.state.update_from_view().remove_connection(id)?; Some(self.controller.disconnect(&ast_to_remove)) @@ -176,7 +182,7 @@ impl Model { } fn nodes_collapsed(&self, collapsed: &[ViewNodeId]) { - self.update_ast( + self.log_action( || { debug!(self.logger, "Collapsing node."); let ids = collapsed.iter().filter_map(|node| self.state.ast_node_id_of_view(*node)); @@ -190,7 +196,7 @@ impl Model { ); } - fn update_ast(&self, f: F, action: &str) + fn log_action(&self, f: F, action: &str) where F: FnOnce() -> Option { if let Some(Err(err)) = f() { error!(self.logger, "Failed to {action} in AST: {err}"); @@ -550,6 +556,7 @@ impl Graph { eval view.on_edge_endpoint_unset(((edge_id,_)) model.connection_removed(*edge_id)); eval view.nodes_collapsed(((nodes, _)) model.nodes_collapsed(nodes)); eval view.enabled_visualization_path(((node_id, path)) model.node_visualization_changed(*node_id, path.clone())); + eval view.node_expression_set(((node_id, expression)) model.node_expression_set(*node_id, expression.clone())); // === Dropping Files === @@ -617,6 +624,7 @@ impl Graph { /// content of a node. For example, the searcher uses this to allow the edit field to have a /// preview that is different from the actual node AST. pub fn allow_expression_auto_updates(&self, id: AstNodeId, allow: bool) { + tracing::debug!("Setting auto updates for {id:?} to {allow}"); self.model.state.allow_expression_auto_updates(id, allow); } } diff --git a/app/gui/src/presenter/graph/state.rs b/app/gui/src/presenter/graph/state.rs index c85e67cb742..1b965519375 100644 --- a/app/gui/src/presenter/graph/state.rs +++ b/app/gui/src/presenter/graph/state.rs @@ -444,6 +444,12 @@ impl<'a> ControllerChange<'a> { }; let mut nodes = self.nodes.borrow_mut(); let displayed = nodes.get_mut_or_create(ast_id); + tracing::debug!( + "Setting node expression from controller: {} -> {}", + displayed.expression, + new_displayed_expr + ); + if displayed.expression != new_displayed_expr { displayed.expression = new_displayed_expr.clone(); let new_expressions = @@ -650,6 +656,21 @@ impl<'a> ViewChange<'a> { None } } + + /// Set the node expression. + pub fn set_node_expression(&self, id: ViewNodeId, expression: String) -> Option { + let mut nodes = self.nodes.borrow_mut(); + let ast_id = nodes.ast_id_of_view(id)?; + let displayed = nodes.get_mut(ast_id)?; + let expression = node_view::Expression::new_plain(expression); + tracing::debug!( + "Setting node expression from view: {} -> {}", + displayed.expression, + expression + ); + let expression_has_changed = displayed.expression != expression; + expression_has_changed.as_some(ast_id) + } } diff --git a/app/gui/src/presenter/project.rs b/app/gui/src/presenter/project.rs index ac73d6b2103..3181b84ac2e 100644 --- a/app/gui/src/presenter/project.rs +++ b/app/gui/src/presenter/project.rs @@ -129,9 +129,15 @@ impl Model { fn editing_aborted(&self) { let searcher = self.searcher.take(); if let Some(searcher) = searcher { - searcher.abort_editing(); + let input_node_view = searcher.input_view(); + if let Some(node) = self.graph.ast_node_of_view(input_node_view) { + self.graph.allow_expression_auto_updates(node, true); + searcher.abort_editing(); + } else { + tracing::warn!("When porting editing the AST node of the node view {input_node_view} could not be found."); + } } else { - warning!(self.logger, "Editing aborted without searcher controller."); + tracing::warn!("Editing aborted without searcher controller."); } } diff --git a/app/gui/src/presenter/searcher.rs b/app/gui/src/presenter/searcher.rs index 24e1776a740..237a6aa79a5 100644 --- a/app/gui/src/presenter/searcher.rs +++ b/app/gui/src/presenter/searcher.rs @@ -97,7 +97,7 @@ impl Model { #[profile(Debug)] fn input_changed(&self, new_input: &str) { if let Err(err) = self.controller.set_input(new_input.to_owned()) { - tracing::error!("Error while setting new searcher input: {}", err); + tracing::error!("Error while setting new searcher input: {err}."); } } @@ -111,7 +111,7 @@ impl Model { Some((self.input_view, new_code_and_trees)) } Err(err) => { - tracing::error!("Error while applying suggestion: {}", err); + tracing::error!("Error while applying suggestion: {err}."); None } } @@ -120,7 +120,9 @@ impl Model { /// Should be called if an entry is selected but not used yet. Only used for the old searcher /// API. fn entry_selected_as_suggestion(&self, entry_id: view::searcher::entry::Id) { - self.controller.preview_entry_as_suggestion(entry_id); + if let Err(error) = self.controller.preview_entry_as_suggestion(entry_id) { + tracing::warn!("Failed to preview entry {entry_id:?} because of error: {error:?}."); + } } fn commit_editing(&self, entry_id: Option) -> Option { @@ -129,7 +131,7 @@ impl Model { None => self.controller.commit_node().map(Some), }; result.unwrap_or_else(|err| { - tracing::error!("Error while executing action: {}", err); + tracing::error!("Error while executing action: {err}."); None }) } @@ -151,7 +153,11 @@ impl Model { /// Should be called if a suggestion is selected but not used yet. fn suggestion_selected(&self, entry_id: list_panel::EntryId) { let suggestion = self.suggestion_for_entry_id(entry_id).unwrap(); - self.controller.preview_suggestion(suggestion); + if let Err(error) = self.controller.preview_suggestion(suggestion) { + tracing::warn!( + "Failed to preview suggestion {entry_id:?} because of error: {error:?}." + ); + } } fn suggestion_accepted( @@ -174,7 +180,7 @@ impl Model { Some((self.input_view, new_code_and_trees)) } Err(err) => { - tracing::error!("Error while applying suggestion: {}", err); + tracing::error!("Error while applying suggestion: {err}."); None } } @@ -188,7 +194,7 @@ impl Model { self.suggestion_accepted(entry_id); } self.controller.commit_node().map(Some).unwrap_or_else(|err| { - tracing::error!("Error while committing node expression: {}", err); + tracing::error!("Error while committing node expression: {err}."); None }) } @@ -404,7 +410,6 @@ impl Searcher { let created_node = graph_controller.add_node(new_node)?; graph.assign_node_view_explicitly(input, created_node); - graph.allow_expression_auto_updates(created_node, false); let source_node = source_node.and_then(|id| graph.ast_node_of_view(id.node)); @@ -425,14 +430,19 @@ impl Searcher { let SearcherParams { input, .. } = parameters; let ast_node = graph.ast_node_of_view(input); - match ast_node { + let mode = match ast_node { Some(node_id) => Ok(Mode::EditNode { node_id }), None => { let (new_node, source_node) = Self::create_input_node(parameters, graph, graph_editor, graph_controller)?; Ok(Mode::NewNode { node_id: new_node, source_node }) } + }; + let target_node = mode.as_ref().map(|mode| mode.node_id()); + if let Ok(target_node) = target_node { + graph.allow_expression_auto_updates(target_node, false); } + mode } /// Setup new, appropriate searcher controller for the edition of `node_view`, and construct @@ -468,8 +478,7 @@ impl Searcher { if source_node.is_none() { if let Err(e) = searcher_controller.set_input("".to_string()) { tracing::error!( - "Failed to clear input when creating searcher for a new node: {:?}", - e + "Failed to clear input when creating searcher for a new node: {e:?}." ); } } @@ -509,6 +518,11 @@ impl Searcher { /// editing finishes. pub fn abort_editing(self) {} + /// Returns the node view that is being edited by the searcher. + pub fn input_view(&self) -> ViewNodeId { + self.model.input_view + } + /// Returns true if the entry under given index is one of the examples. pub fn is_entry_an_example(&self, entry: view::searcher::entry::Id) -> bool { use crate::controller::searcher::action::Action::Example; diff --git a/app/gui/view/src/project.rs b/app/gui/view/src/project.rs index 32317dc0b11..3976e98de08 100644 --- a/app/gui/view/src/project.rs +++ b/app/gui/view/src/project.rs @@ -623,7 +623,6 @@ impl View { graph.deselect_all_nodes(); graph.select_node(node); }); - eval adding_aborted ((node) graph.remove_node(node)); // === Editing === diff --git a/build-config.yaml b/build-config.yaml index 8973dbd9e4b..027a192fa2a 100644 --- a/build-config.yaml +++ b/build-config.yaml @@ -1,6 +1,6 @@ # Options intended to be common for all developers. -wasm-size-limit: 14.45 MiB +wasm-size-limit: 14.46 MiB required-versions: cargo-watch: ^8.1.1