From 9f4a5f90c0e5e96015d70bbff7317ff3a0532748 Mon Sep 17 00:00:00 2001 From: Ilya Bogdanov Date: Fri, 11 Aug 2023 18:45:15 +0400 Subject: [PATCH] Correctly display connections to lambdas (#7550) Closes #7261 It's impossible to connect to the lambda arguments, but they are displayed as in code, and the correct span tree is generated for the lambda body. (hence you can connect to items inside) https://github.com/enso-org/enso/assets/6566674/60af6413-e1b9-4e8c-a958-2906b5534d62 --- CHANGELOG.md | 3 + Cargo.lock | 23 +++++ app/gui/language/span-tree/Cargo.toml | 1 + app/gui/language/span-tree/src/builder.rs | 6 ++ app/gui/language/span-tree/src/generate.rs | 106 +++++++++++++++++++-- app/gui/view/src/notification.rs | 1 - app/gui/view/src/notification/logged.rs | 1 - lib/rust/text/src/unit.rs | 7 ++ 8 files changed, 138 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79c3f4fd2e7e..cf5d68b741a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -215,6 +215,8 @@ the selected entry in the component browser. Clear separating lines between method arguments were added. The node selection was made easier with additional thick interactive selection border. +- [Connections to lamdas are displayed correctly][7550]. It is possible to drag + a connection to any expression inside the lambda body. [5910]: https://github.com/enso-org/enso/pull/5910 [6279]: https://github.com/enso-org/enso/pull/6279 @@ -239,6 +241,7 @@ [7372]: https://github.com/enso-org/enso/pull/7372 [7337]: https://github.com/enso-org/enso/pull/7337 [7311]: https://github.com/enso-org/enso/pull/7311 +[7550]: https://github.com/enso-org/enso/pull/7550 #### EnsoGL (rendering engine) diff --git a/Cargo.lock b/Cargo.lock index 698c7931ed65..e67c2f4365fd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1717,6 +1717,12 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2ea6706d74fca54e15f1d40b5cf7fe7f764aaec61352a9fcec58fe27e042fc8" +[[package]] +name = "diff" +version = "0.1.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "56254986775e3233ffa9c4d7d3faaf6d36a2c09d30b20687e9f88bc8bafc16c8" + [[package]] name = "difference" version = "2.0.0" @@ -5600,6 +5606,16 @@ dependencies = [ "termtree", ] +[[package]] +name = "pretty_assertions" +version = "1.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "af7cee1a6c8a5b9208b3cb1061f10c0cb689087b3d8ce85fb9d2dd7a29b6ba66" +dependencies = [ + "diff", + "yansi", +] + [[package]] name = "pretty_env_logger" version = "0.4.0" @@ -6497,6 +6513,7 @@ dependencies = [ "enso-text", "failure", "parser", + "pretty_assertions", "wasm-bindgen-test", ] @@ -7774,6 +7791,12 @@ version = "0.13.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "114ba2b24d2167ef6d67d7d04c8cc86522b87f490025f39f0303b7db5bf5e3d8" +[[package]] +name = "yansi" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09041cd90cf85f7f8b2df60c646f853b7f535ce68f85244eb6731cf89fa498ec" + [[package]] name = "zeroize" version = "1.5.7" diff --git a/app/gui/language/span-tree/Cargo.toml b/app/gui/language/span-tree/Cargo.toml index 9c422a7a3b1b..0a8774d0d381 100644 --- a/app/gui/language/span-tree/Cargo.toml +++ b/app/gui/language/span-tree/Cargo.toml @@ -15,3 +15,4 @@ parser = { path = "../parser" } [dev-dependencies] wasm-bindgen-test = { workspace = true } +pretty_assertions = "1.4" diff --git a/app/gui/language/span-tree/src/builder.rs b/app/gui/language/span-tree/src/builder.rs index 9c398227c2ec..cdfa5150ff0e 100644 --- a/app/gui/language/span-tree/src/builder.rs +++ b/app/gui/language/span-tree/src/builder.rs @@ -97,4 +97,10 @@ impl ChildBuilder { self.built.node.ast_id = Some(id); self } + + /// Set tree type for this node. + pub fn set_tree_type(mut self, r#type: Option) -> Self { + self.built.node.tree_type = r#type; + self + } } diff --git a/app/gui/language/span-tree/src/generate.rs b/app/gui/language/span-tree/src/generate.rs index 5357e12c77e8..264a8cacaf20 100644 --- a/app/gui/language/span-tree/src/generate.rs +++ b/app/gui/language/span-tree/src/generate.rs @@ -344,8 +344,7 @@ fn generate_node_for_ast( match ast.shape() { ast::Shape::Prefix(_) => ast::prefix::Chain::from_ast(ast).unwrap().generate_node(kind, context), - ast::Shape::Tree(tree) if tree.type_info != ast::TreeType::Lambda => - tree_generate_node(tree, kind, context, ast.id), + ast::Shape::Tree(tree) => tree_generate_node(tree, kind, context, ast.id), ast::Shape::Block(block) => block_generate_node(block, kind, context, ast.id), _ => { let size = (ast.repr_len().value as i32).byte_diff(); @@ -818,11 +817,44 @@ fn generate_trailing_expected_arguments( }) } +/// A single child node produced out of the lambda argument and the `->` token. +#[derive(Debug)] +struct FoldedLambdaArguments { + /// Both the lambda argument and the `->` token, as a single [`node::Kind::Token`] node. + child: node::Child, + /// The number of tree nodes that were folded into the `child`. + nodes_replaced: usize, +} - -// ========================= -// === SpanTree for Tree === -// ========================= +/// Fold the lambda arguments into a single [`node::Kind::Token`] node. +/// It is needed to ignore lambda arguments as connection targets, but still generate a valid +/// SpanTree from the lambda body. +fn fold_lambda_arguments(tree: &ast::Tree) -> FoldedLambdaArguments { + let is_arrow = |span_info| matches!(span_info, SpanSeed::Token(ast::SpanSeedToken { token }) if token == "->"); + let arrow_index = tree.span_info.iter().cloned().position(is_arrow).unwrap_or(0); + let bytes_till_body = tree + .span_info + .iter() + .take(arrow_index + 1) + .map(|raw_span_info| match raw_span_info { + SpanSeed::Space(ast::SpanSeedSpace { space }) => ByteDiff::from(space), + SpanSeed::Token(ast::SpanSeedToken { token }) => ByteDiff::from(token.len()), + SpanSeed::Child(ast::SpanSeedChild { node }) => node.repr_len().to_diff(), + }) + .sum::(); + let size = bytes_till_body; + let kind = node::Kind::Token; + let node = Node::new().with_kind(kind).with_size(size); + let ast_crumbs = vec![TreeCrumb { index: 0 }.into()]; + let nodes_replaced = arrow_index + 1; + let child = node::Child { + node, + parent_offset: ByteDiff::from(0), + sibling_offset: ByteDiff::from(0), + ast_crumbs, + }; + FoldedLambdaArguments { child, nodes_replaced } +} fn tree_generate_node( tree: &ast::Tree, @@ -846,7 +878,19 @@ fn tree_generate_node( let last_token_index = tree.span_info.iter().rposition(|span| matches!(span, SpanSeed::Token(_))); - for (index, raw_span_info) in tree.span_info.iter().enumerate() { + + // If the node is a lambda, we fold the lambda arguments into a single child node, + // and then continue handling the lambda body as usual. + let skip = if tree.type_info == ast::TreeType::Lambda { + let FoldedLambdaArguments { child, nodes_replaced } = fold_lambda_arguments(tree); + parent_offset += child.node.size; + children.push(child); + nodes_replaced + } else { + 0 + }; + for (index, raw_span_info) in tree.span_info.iter().skip(skip).enumerate() { + let index = index + skip; match raw_span_info { SpanSeed::Space(ast::SpanSeedSpace { space }) => { parent_offset += ByteDiff::from(space); @@ -985,6 +1029,7 @@ mod test { use ast::Crumbs; use ast::IdMap; use parser::Parser; + use pretty_assertions::assert_eq; /// A helper function which removes information about expression id from thw tree rooted at @@ -1150,6 +1195,10 @@ mod test { #[test] fn generating_span_tree_for_lambda() { let parser = Parser::new(); + + + // === Simple lambda === + let ast = parser.parse_line_ast("foo a-> b + c").unwrap(); let mut tree: SpanTree = ast.generate_tree(&context::Empty).unwrap(); clear_expression_ids(&mut tree.root); @@ -1158,10 +1207,51 @@ mod test { let expected = TreeBuilder::new(13) .add_leaf(0, 3, node::Kind::Operation, PrefixCrumb::Func) .add_empty_child(3, BeforeArgument(0)) - .add_leaf(4, 9, node::Kind::prefix_argument(), PrefixCrumb::Arg) + .add_child(4, 9, node::Kind::prefix_argument(), PrefixCrumb::Arg) + .set_tree_type(Some(ast::TreeType::Lambda)) + .add_leaf(0, 3, node::Kind::Token, TreeCrumb { index: 0 }) + .add_child(4, 5, node::Kind::argument(), TreeCrumb { index: 3 }) + .add_empty_child(0, BeforeArgument(0)) + .add_leaf(0, 1, node::Kind::argument(), InfixCrumb::LeftOperand) + .add_leaf(2, 1, node::Kind::Operation, InfixCrumb::Operator) + .add_empty_child(3, BeforeArgument(1)) + .add_leaf(4, 1, node::Kind::argument(), InfixCrumb::RightOperand) + .add_empty_child(5, Append) + .done() + .done() .add_empty_child(13, Append) .build(); + assert_eq!(expected, tree); + + // === Lambda with two arguments === + + let ast = parser.parse_line_ast("foo a->b-> a + b").unwrap(); + let mut tree: SpanTree = ast.generate_tree(&context::Empty).unwrap(); + clear_expression_ids(&mut tree.root); + clear_parameter_infos(&mut tree.root); + + let expected = TreeBuilder::new(16) + .add_leaf(0, 3, node::Kind::Operation, PrefixCrumb::Func) + .add_empty_child(3, BeforeArgument(0)) + .add_child(4, 12, node::Kind::prefix_argument(), PrefixCrumb::Arg) + .set_tree_type(Some(ast::TreeType::Lambda)) + .add_leaf(0, 3, node::Kind::Token, TreeCrumb { index: 0 }) + .add_child(3, 9, node::Kind::argument(), TreeCrumb { index: 2 }) + .set_tree_type(Some(ast::TreeType::Lambda)) + .add_leaf(0, 3, node::Kind::Token, TreeCrumb { index: 0 }) + .add_child(4, 5, node::Kind::argument(), TreeCrumb { index: 3 }) + .add_empty_child(0, BeforeArgument(0)) + .add_leaf(0, 1, node::Kind::argument(), InfixCrumb::LeftOperand) + .add_leaf(2, 1, node::Kind::Operation, InfixCrumb::Operator) + .add_empty_child(3, BeforeArgument(1)) + .add_leaf(4, 1, node::Kind::argument(), InfixCrumb::RightOperand) + .add_empty_child(5, Append) + .done() + .done() + .done() + .add_empty_child(16, Append) + .build(); assert_eq!(expected, tree); } diff --git a/app/gui/view/src/notification.rs b/app/gui/view/src/notification.rs index 39dfaf25048f..f217e393e20c 100644 --- a/app/gui/view/src/notification.rs +++ b/app/gui/view/src/notification.rs @@ -10,7 +10,6 @@ use crate::notification::logged::UpdateOptions; use ensogl::application::Application; - // ============== // === Export === // ============== diff --git a/app/gui/view/src/notification/logged.rs b/app/gui/view/src/notification/logged.rs index bcca1332fcbf..c2a316a4ae08 100644 --- a/app/gui/view/src/notification/logged.rs +++ b/app/gui/view/src/notification/logged.rs @@ -16,7 +16,6 @@ use crate::notification::js::HandleJsError; use uuid::Uuid; - // ============== // === Export === // ============== diff --git a/lib/rust/text/src/unit.rs b/lib/rust/text/src/unit.rs index 84f95ea4d98e..5114239a7922 100644 --- a/lib/rust/text/src/unit.rs +++ b/lib/rust/text/src/unit.rs @@ -5,6 +5,7 @@ use crate::index::*; use crate::prelude::*; use enso_types::unit; +use std::iter::Sum; @@ -118,6 +119,12 @@ impl SubAssign for ByteDiff { } } +impl Sum for ByteDiff { + fn sum>(iter: I) -> Self { + iter.fold(ByteDiff(0), |acc, x| acc + x) + } +} + // ================