Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show Visualisation Preview when Selecting Item on Searcher #3691

4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand Down
139 changes: 95 additions & 44 deletions app/gui/src/controller/searcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No docs, and the name does not explain what it is.



// ==============
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -648,37 +649,48 @@ 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<String> {
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<ast::prefix::Chain> {
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),
sast: ast::Shifted::new(
pattern_offset.max(MINIMUM_PATTERN_OFFSET),
added_ast,
Comment on lines +684 to +686
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

export to var to make it shorter.

),
prefix_id: default(),
};
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.
Expand All @@ -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),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make sure: is this restored after editing aborting?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it is.

)?;
tracing::debug!("Previewing expression: \"{:?}\".", expression);
self.graph.graph().set_expression(self.mode.node_id(), expression)?;

Ok(())
}

/// Execute given action.
Expand Down Expand Up @@ -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(())
}

Expand Down Expand Up @@ -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)?;
Expand All @@ -837,6 +868,16 @@ impl Searcher {
Ok(node_id)
}

fn get_expression(&self, input_chain: Option<ast::Shifted<ast::prefix::Chain>>) -> 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
Expand Down Expand Up @@ -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,
});
}),
)
}
Expand Down Expand Up @@ -1311,14 +1356,21 @@ 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,
&previous_expression
);
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(())
Expand All @@ -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 {
Expand All @@ -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}"
)
});
}
Expand Down Expand Up @@ -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,
})
);
}),
Expand Down
5 changes: 4 additions & 1 deletion app/gui/src/model/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<MethodId>,
},
/// The node was created and did not previously exist.
Created,
Expand Down
22 changes: 15 additions & 7 deletions app/gui/src/presenter/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -125,7 +125,7 @@ impl Model {
}

fn node_visualization_changed(&self, id: ViewNodeId, path: Option<visualization_view::Path>) {
self.update_ast(
self.log_action(
|| {
let ast_id =
self.state.update_from_view().set_node_visualization(id, path.clone())?;
Expand All @@ -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))
Expand All @@ -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)?;
Expand All @@ -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))
Expand All @@ -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));
Expand All @@ -190,7 +196,7 @@ impl Model {
);
}

fn update_ast<F>(&self, f: F, action: &str)
fn log_action<F>(&self, f: F, action: &str)
where F: FnOnce() -> Option<FallibleResult> {
if let Some(Err(err)) = f() {
error!(self.logger, "Failed to {action} in AST: {err}");
Expand Down Expand Up @@ -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 ===
Expand Down Expand Up @@ -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);
}
}
Expand Down
Loading