From b09c6ae51a1c7f7be81fd614feed6c4c468e92fc Mon Sep 17 00:00:00 2001 From: Michael Mauderer Date: Mon, 5 Sep 2022 15:26:05 +0100 Subject: [PATCH 01/10] Implement working preview of selected item in Searcher. --- app/gui/src/controller/searcher.rs | 63 +++++++++++++++++++++++++--- app/gui/src/presenter/graph.rs | 22 ++++++---- app/gui/src/presenter/graph/state.rs | 25 +++++++++++ app/gui/src/presenter/project.rs | 2 + app/gui/src/presenter/searcher.rs | 21 ++++++++-- app/gui/view/src/project.rs | 1 - 6 files changed, 117 insertions(+), 17 deletions(-) diff --git a/app/gui/src/controller/searcher.rs b/app/gui/src/controller/searcher.rs index 8fb938bb188e..92d5d9e2b350 100644 --- a/app/gui/src/controller/searcher.rs +++ b/app/gui/src/controller/searcher.rs @@ -696,15 +696,68 @@ impl Searcher { } /// Preview the suggestion in the searcher. - pub fn preview_entry_as_suggestion(&self, index: usize) { + pub fn preview_entry_as_suggestion(&self, index: usize) -> FallibleResult { tracing::debug!("Previewing entry: {:?}", index); - //TODO[MM] the actual functionality here will be implemented as part of task #182634050. + 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 = match self.data.borrow_mut().input.expression.clone() { + 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 expr_and_method = || { + let input_chain = Some(new_expression); + + 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 (expression, intended_method) = expr_and_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. diff --git a/app/gui/src/presenter/graph.rs b/app/gui/src/presenter/graph.rs index 0d81baada535..136ea748444b 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 c85e67cb742d..614d750571ec 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,25 @@ 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 + ); + if displayed.expression != expression { + displayed.expression = expression; + Some(ast_id) + } else { + None + } + } } diff --git a/app/gui/src/presenter/project.rs b/app/gui/src/presenter/project.rs index ac73d6b2103c..18d115bd0dbc 100644 --- a/app/gui/src/presenter/project.rs +++ b/app/gui/src/presenter/project.rs @@ -129,6 +129,8 @@ impl Model { fn editing_aborted(&self) { let searcher = self.searcher.take(); if let Some(searcher) = searcher { + let node = self.graph.ast_node_of_view(searcher.input_view()).unwrap(); + self.graph.allow_expression_auto_updates(node, true); searcher.abort_editing(); } else { warning!(self.logger, "Editing aborted without searcher controller."); diff --git a/app/gui/src/presenter/searcher.rs b/app/gui/src/presenter/searcher.rs index 24e1776a740f..1e230aa1f0ea 100644 --- a/app/gui/src/presenter/searcher.rs +++ b/app/gui/src/presenter/searcher.rs @@ -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 { @@ -151,7 +153,9 @@ 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( @@ -404,7 +408,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 +428,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 @@ -509,6 +517,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 ff82fdfbce90..023406d5e1ba 100644 --- a/app/gui/view/src/project.rs +++ b/app/gui/view/src/project.rs @@ -621,7 +621,6 @@ impl View { graph.deselect_all_nodes(); graph.select_node(node); }); - eval adding_aborted ((node) graph.remove_node(node)); // === Editing === From c200c14121beaf79c158098942deb67b6e28a779 Mon Sep 17 00:00:00 2001 From: Michael Mauderer Date: Wed, 7 Sep 2022 15:10:42 +0100 Subject: [PATCH 02/10] Update changelog. --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4aaee0784caf..e56040315766 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,9 @@ new node is created.][3645] - [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) @@ -305,6 +308,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 #### Enso Compiler From 0853b2715da22cf36fadc10747e8c8341e8d4a68 Mon Sep 17 00:00:00 2001 From: Michael Mauderer Date: Wed, 7 Sep 2022 15:34:19 +0100 Subject: [PATCH 03/10] Prettier. --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e56040315766..3c3718da013d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,8 +62,8 @@ new node is created.][3645] - [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 +- [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) From 33bd77495a611751f0777fa93ad301b6ad7aef39 Mon Sep 17 00:00:00 2001 From: Michael Mauderer Date: Mon, 12 Sep 2022 11:17:27 +0100 Subject: [PATCH 04/10] Implement review feedback. --- app/gui/src/controller/searcher.rs | 6 +++++- app/gui/src/presenter/project.rs | 12 ++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/app/gui/src/controller/searcher.rs b/app/gui/src/controller/searcher.rs index 92d5d9e2b350..5a8dc9e8ce27 100644 --- a/app/gui/src/controller/searcher.rs +++ b/app/gui/src/controller/searcher.rs @@ -661,8 +661,12 @@ impl Searcher { ast::Shifted::new(pattern_offset, ast) } Some(mut expression) => { + const MINIMUM_PATTERN_OFFSET: usize = 1; let new_argument = ast::prefix::Argument { - sast: ast::Shifted::new(pattern_offset.max(1), added_ast), + sast: ast::Shifted::new( + pattern_offset.max(MINIMUM_PATTERN_OFFSET), + added_ast, + ), prefix_id: default(), }; expression.args.push(new_argument); diff --git a/app/gui/src/presenter/project.rs b/app/gui/src/presenter/project.rs index 18d115bd0dbc..34cc27e5705a 100644 --- a/app/gui/src/presenter/project.rs +++ b/app/gui/src/presenter/project.rs @@ -129,11 +129,15 @@ impl Model { fn editing_aborted(&self) { let searcher = self.searcher.take(); if let Some(searcher) = searcher { - let node = self.graph.ast_node_of_view(searcher.input_view()).unwrap(); - self.graph.allow_expression_auto_updates(node, true); - 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} could not be found."); + } } else { - warning!(self.logger, "Editing aborted without searcher controller."); + tracing::warn!("Editing aborted without searcher controller."); } } From 3afd1ca20c821e03c4cfc1c783378ea2b33a6abe Mon Sep 17 00:00:00 2001 From: Michael Mauderer Date: Mon, 12 Sep 2022 11:19:49 +0100 Subject: [PATCH 05/10] Implement review feedback. --- app/gui/src/controller/searcher.rs | 7 +++++-- app/gui/src/presenter/project.rs | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/gui/src/controller/searcher.rs b/app/gui/src/controller/searcher.rs index 5a8dc9e8ce27..8795d242dfec 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; // ============== @@ -661,7 +662,6 @@ impl Searcher { ast::Shifted::new(pattern_offset, ast) } Some(mut expression) => { - const MINIMUM_PATTERN_OFFSET: usize = 1; let new_argument = ast::prefix::Argument { sast: ast::Shifted::new( pattern_offset.max(MINIMUM_PATTERN_OFFSET), @@ -732,7 +732,10 @@ impl Searcher { } Some(mut expression) => { let new_argument = ast::prefix::Argument { - sast: ast::Shifted::new(pattern_offset.max(1), added_ast), + sast: ast::Shifted::new( + pattern_offset.max(MINIMUM_PATTERN_OFFSET), + added_ast, + ), prefix_id: default(), }; expression.args.push(new_argument); diff --git a/app/gui/src/presenter/project.rs b/app/gui/src/presenter/project.rs index 34cc27e5705a..3181b84ac2ed 100644 --- a/app/gui/src/presenter/project.rs +++ b/app/gui/src/presenter/project.rs @@ -134,7 +134,7 @@ impl Model { 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} could not be found."); + tracing::warn!("When porting editing the AST node of the node view {input_node_view} could not be found."); } } else { tracing::warn!("Editing aborted without searcher controller."); From 4a8935e6a662464fab1a73567df36661c5825846 Mon Sep 17 00:00:00 2001 From: Michael Mauderer Date: Mon, 12 Sep 2022 11:27:34 +0100 Subject: [PATCH 06/10] Implement review feedback. --- app/gui/src/presenter/graph/state.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/app/gui/src/presenter/graph/state.rs b/app/gui/src/presenter/graph/state.rs index 614d750571ec..1b9655193750 100644 --- a/app/gui/src/presenter/graph/state.rs +++ b/app/gui/src/presenter/graph/state.rs @@ -668,12 +668,8 @@ impl<'a> ViewChange<'a> { displayed.expression, expression ); - if displayed.expression != expression { - displayed.expression = expression; - Some(ast_id) - } else { - None - } + let expression_has_changed = displayed.expression != expression; + expression_has_changed.as_some(ast_id) } } From c85e8ac597269d2205bbbc4d38297687d9c8658e Mon Sep 17 00:00:00 2001 From: Michael Mauderer Date: Tue, 13 Sep 2022 12:49:17 +0100 Subject: [PATCH 07/10] Implement Adam's review feedback. --- app/gui/src/controller/searcher.rs | 64 +++++++++++++++--------------- app/gui/src/model/module.rs | 5 ++- 2 files changed, 37 insertions(+), 32 deletions(-) diff --git a/app/gui/src/controller/searcher.rs b/app/gui/src/controller/searcher.rs index 8795d242dfec..dc8de861a93a 100644 --- a/app/gui/src/controller/searcher.rs +++ b/app/gui/src/controller/searcher.rs @@ -728,7 +728,7 @@ impl Searcher { let new_expression = match self.data.borrow_mut().input.expression.clone() { None => { let ast = ast::prefix::Chain::from_ast_non_strict(&added_ast); - ast::Shifted::new(pattern_offset, ast) + Some(ast::Shifted::new(pattern_offset, ast)) } Some(mut expression) => { let new_argument = ast::prefix::Argument { @@ -739,22 +739,11 @@ impl Searcher { prefix_id: default(), }; expression.args.push(new_argument); - expression + Some(expression) } }; - let expr_and_method = || { - let input_chain = Some(new_expression); - - 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 (expression, intended_method) = expr_and_method(); + let expression = self.get_expression(new_expression); + let intended_method = self.intended_method(); self.graph.graph().module.with_node_metadata( self.mode.node_id(), @@ -864,24 +853,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(); + let input_chain = self.data.borrow().input.as_prefix_chain(self.ide.parser()); // 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 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)?; @@ -897,6 +877,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 @@ -1328,7 +1318,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, + }); }), ) } @@ -1371,7 +1365,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, @@ -1379,6 +1373,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(()) @@ -2472,7 +2473,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 7bc635fd9fda..3c561043d01f 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, From 2794f3f7f49991d246be50cd17bbfdefcc2f3100 Mon Sep 17 00:00:00 2001 From: Michael Mauderer Date: Wed, 14 Sep 2022 13:17:28 +0100 Subject: [PATCH 08/10] Code cleanup based on review feedback. --- app/gui/src/controller/searcher.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/gui/src/controller/searcher.rs b/app/gui/src/controller/searcher.rs index dc8de861a93a..7ec1767b54cd 100644 --- a/app/gui/src/controller/searcher.rs +++ b/app/gui/src/controller/searcher.rs @@ -725,7 +725,7 @@ impl Searcher { 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.clone() { + let new_expression_chain = match self.data.borrow_mut().input.expression.clone() { None => { let ast = ast::prefix::Chain::from_ast_non_strict(&added_ast); Some(ast::Shifted::new(pattern_offset, ast)) @@ -742,7 +742,7 @@ impl Searcher { Some(expression) } }; - let expression = self.get_expression(new_expression); + let expression = self.get_expression(new_expression_chain); let intended_method = self.intended_method(); self.graph.graph().module.with_node_metadata( @@ -854,14 +854,14 @@ impl Searcher { guard.prevent_revert() } - let node_id = self.mode.node_id(); - let input_chain = self.data.borrow().input.as_prefix_chain(self.ide.parser()); // 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 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)?; From 6babdff4fad8c2ae66fa7a43bdd414063a6e3d88 Mon Sep 17 00:00:00 2001 From: Michael Mauderer Date: Thu, 15 Sep 2022 11:27:37 +0100 Subject: [PATCH 09/10] Review feedback. --- app/gui/src/controller/searcher.rs | 75 +++++++++++++----------------- app/gui/src/presenter/searcher.rs | 19 ++++---- 2 files changed, 42 insertions(+), 52 deletions(-) diff --git a/app/gui/src/controller/searcher.rs b/app/gui/src/controller/searcher.rs index 7ec1767b54cd..89401bc9f44a 100644 --- a/app/gui/src/controller/searcher.rs +++ b/app/gui/src/controller/searcher.rs @@ -607,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(); @@ -649,14 +649,32 @@ 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() { + let new_expression_chain = self.create_new_expression_chain(added_ast, pattern_offset); + let new_parsed_input = ParsedInput { + expression: Some(new_expression_chain), + pattern_offset: 1, + pattern: "".to_string(), + }; + let new_input = new_parsed_input.repr(); + self.data.borrow_mut().input = new_parsed_input; + self.data.borrow_mut().fragments_added_by_picking.push(picked_completion); + self.reload_list(); + 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) @@ -672,17 +690,7 @@ impl Searcher { expression.args.push(new_argument); expression } - }; - let new_parsed_input = ParsedInput { - expression: Some(new_expression), - pattern_offset: 1, - pattern: "".to_string(), - }; - let new_input = new_parsed_input.repr(); - self.data.borrow_mut().input = new_parsed_input; - self.data.borrow_mut().fragments_added_by_picking.push(picked_completion); - self.reload_list(); - Ok(new_input) + } } /// Use action at given index as a suggestion. The exact outcome depends on the action's type. @@ -701,7 +709,7 @@ impl Searcher { /// Preview the suggestion in the searcher. pub fn preview_entry_as_suggestion(&self, index: usize) -> FallibleResult { - tracing::debug!("Previewing entry: {:?}", index); + tracing::debug!("Previewing entry: {index:?}."); let error = || NoSuchAction { index }; let suggestion = { let data = self.data.borrow(); @@ -717,40 +725,23 @@ 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); + 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); + 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 = match self.data.borrow_mut().input.expression.clone() { - None => { - let ast = ast::prefix::Chain::from_ast_non_strict(&added_ast); - Some(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); - Some(expression) - } - }; - let expression = self.get_expression(new_expression_chain); + 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); - + tracing::debug!("Previewing expression: \"{:?}\".", expression); self.graph.graph().set_expression(self.mode.node_id(), expression)?; Ok(()) @@ -824,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(()) } @@ -1391,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 { @@ -1401,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}" ) }); } diff --git a/app/gui/src/presenter/searcher.rs b/app/gui/src/presenter/searcher.rs index 1e230aa1f0ea..237a6aa79a50 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 } } @@ -121,7 +121,7 @@ impl Model { /// API. fn entry_selected_as_suggestion(&self, entry_id: view::searcher::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:?}"); + tracing::warn!("Failed to preview entry {entry_id:?} because of error: {error:?}."); } } @@ -131,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 }) } @@ -154,7 +154,9 @@ impl Model { fn suggestion_selected(&self, entry_id: list_panel::EntryId) { let suggestion = self.suggestion_for_entry_id(entry_id).unwrap(); if let Err(error) = self.controller.preview_suggestion(suggestion) { - tracing::warn!("Failed to preview suggestion {entry_id:?} because of error: {error:?}"); + tracing::warn!( + "Failed to preview suggestion {entry_id:?} because of error: {error:?}." + ); } } @@ -178,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 } } @@ -192,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 }) } @@ -476,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:?}." ); } } From 27651d0b3296efb74ed044aa3ada8dd6405070cf Mon Sep 17 00:00:00 2001 From: Michael Mauderer Date: Thu, 15 Sep 2022 12:47:11 +0100 Subject: [PATCH 10/10] Bump wasm-size. --- build-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build-config.yaml b/build-config.yaml index 8973dbd9e4b9..027a192fa2ad 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