Show Visualisation Preview when Selecting Item on Searcher (#3691)

Implements [#182634050](https://www.pivotaltracker.com/story/show/182634050).

Enables visualization previews for selections in the searcher.

Works for the old Searcher and the new Component Browser, but it was easier to show the examples for the old searcher, as the suggestions seem to be better/faster available currently (this will improve with the new implementation using better strings and the new GridView).

https://user-images.githubusercontent.com/1428930/188898560-fc4f412a-1529-49f7-9958-28bf5e01c001.mp4

Aborting an edit now also correctly reverts a node.


https://user-images.githubusercontent.com/1428930/188898549-ddd41294-2571-4e2e-b6d5-909cbf71de04.mp4
This commit is contained in:
Michael Mauderer 2022-09-16 18:05:40 +02:00 committed by GitHub
parent 0e5df935d3
commit 545e1d7ce9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 175 additions and 69 deletions

View File

@ -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

View File

@ -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<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() {
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<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(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<String> {
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<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
@ -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,
})
);
}),

View File

@ -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,

View File

@ -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<visualization_view::Path>) {
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<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}");
@ -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);
}
}

View File

@ -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<AstNodeId> {
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)
}
}

View File

@ -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.");
}
}

View File

@ -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<view::searcher::entry::Id>) -> Option<AstNodeId> {
@ -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;

View File

@ -623,7 +623,6 @@ impl View {
graph.deselect_all_nodes();
graph.select_node(node);
});
eval adding_aborted ((node) graph.remove_node(node));
// === Editing ===

View File

@ -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