diff --git a/CHANGELOG.md b/CHANGELOG.md index a9b920916bc0..fc71e96fbc7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,6 +86,8 @@ present only when the argument is of type that has a predefined set of values. - [Separate component browser navigator sections for modules imported from different namespaces][4044] +- [Added contextual suggestions to argument dropdowns][4072]. Dropdowns will now + contain suggestions which are based on evaluated data. #### EnsoGL (rendering engine) @@ -462,6 +464,7 @@ [4063]: https://github.com/enso-org/enso/pull/4063 [4078]: https://github.com/enso-org/enso/pull/4078 [4097]: https://github.com/enso-org/enso/pull/4097 +[4072]: https://github.com/enso-org/enso/pull/4072 #### Enso Compiler diff --git a/app/gui/controller/double-representation/src/node.rs b/app/gui/controller/double-representation/src/node.rs index 6d48a0f3b830..307434a9342b 100644 --- a/app/gui/controller/double-representation/src/node.rs +++ b/app/gui/controller/double-representation/src/node.rs @@ -24,7 +24,7 @@ use std::cmp::Ordering; -/// Node Id is the Ast Id attached to the node's expression. +/// Node Id is the AST ID attached to the node's expression. pub type Id = ast::Id; @@ -224,7 +224,7 @@ impl NodeInfo { expression_id_matches() || doc_comment_id_matches() } - /// Get the ast id of the line with the node comment (if present). + /// Get the AST ID of the line with the node comment (if present). pub fn doc_comment_id(&self) -> Option { self.documentation.as_ref().and_then(|comment| comment.ast().id()) } @@ -245,7 +245,7 @@ impl NodeInfo { /// Representation of the main line of the node (as opposed to a documentation line). /// /// Each node must have exactly one main line. -/// Main line always contains an expression, either directly or under binding. The expression id +/// Main line always contains an expression, either directly or under binding. The expression ID /// must be set and it serves as the whole node's expression. #[derive(Clone, Debug)] #[allow(missing_docs)] @@ -419,7 +419,7 @@ impl MainLine { } } - /// Modify expression, preserving the AST id. + /// Modify expression, preserving the AST ID. fn modify_expression(&mut self, f: impl FnOnce(&mut Ast)) { let id = self.id(); match self { @@ -431,7 +431,7 @@ impl MainLine { self.set_id(id); } - /// Add [`SKIP`] macro call to the AST. Preserves the expression id and [`FREEZE`] macro calls. + /// Add [`SKIP`] macro call to the AST. Preserves the expression ID and [`FREEZE`] macro calls. fn add_skip_macro(&mut self) { self.modify_expression(|ast| { prepend_with_macro(ast, SKIP_MACRO_IDENTIFIER); @@ -439,7 +439,7 @@ impl MainLine { self.macros_info_mut().skip = true; } - /// Remove [`SKIP`] macro call from the AST. Preserves the expression id and [`FREEZE`] macro + /// Remove [`SKIP`] macro call from the AST. Preserves the expression ID and [`FREEZE`] macro /// calls. fn remove_skip_macro(&mut self) { self.modify_expression(|ast| { @@ -448,7 +448,7 @@ impl MainLine { self.macros_info_mut().skip = false; } - /// Add [`FREEZE`] macro call to the AST. Preserves the expression id and [`SKIP`] macro calls. + /// Add [`FREEZE`] macro call to the AST. Preserves the expression ID and [`SKIP`] macro calls. fn add_freeze_macro(&mut self) { self.modify_expression(|ast| { *ast = preserving_skip(ast, |ast| prepend_with_macro(ast, FREEZE_MACRO_IDENTIFIER)); @@ -456,7 +456,7 @@ impl MainLine { self.macros_info_mut().freeze = true; } - /// Remove [`FREEZE`] macro call from the AST. Preserves the expression id and [`SKIP`] macro + /// Remove [`FREEZE`] macro call from the AST. Preserves the expression ID and [`SKIP`] macro /// calls. fn remove_freeze_macro(&mut self) { self.modify_expression(|ast| { diff --git a/app/gui/controller/engine-protocol/src/language_server/types.rs b/app/gui/controller/engine-protocol/src/language_server/types.rs index fc8095dff583..2c625af5cb0d 100644 --- a/app/gui/controller/engine-protocol/src/language_server/types.rs +++ b/app/gui/controller/engine-protocol/src/language_server/types.rs @@ -1117,8 +1117,8 @@ impl FieldUpdate { } #[derive(Clone, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)] -#[serde(rename_all = "camelCase")] #[allow(missing_docs)] +#[serde(tag = "type")] pub enum SuggestionArgumentUpdate { #[serde(rename_all = "camelCase")] Add { index: usize, argument: SuggestionEntryArgument }, diff --git a/app/gui/language/ast/impl/src/id_map.rs b/app/gui/language/ast/impl/src/id_map.rs index 5dcbf0fd2fe3..479ed74b0790 100644 --- a/app/gui/language/ast/impl/src/id_map.rs +++ b/app/gui/language/ast/impl/src/id_map.rs @@ -36,8 +36,10 @@ impl IdMap { self.vec.push((span.into(), id)); } /// Generate random Uuid for span. - pub fn generate(&mut self, span: impl Into>) { - self.vec.push((span.into(), Uuid::new_v4())); + pub fn generate(&mut self, span: impl Into>) -> Uuid { + let uuid = Uuid::new_v4(); + self.vec.push((span.into(), uuid)); + uuid } } diff --git a/app/gui/language/ast/impl/src/known.rs b/app/gui/language/ast/impl/src/known.rs index 6c4e43a3f915..2fae1652fa9e 100644 --- a/app/gui/language/ast/impl/src/known.rs +++ b/app/gui/language/ast/impl/src/known.rs @@ -41,7 +41,7 @@ impl KnownAst { KnownAst { ast, phantom: default() } } - /// Gets AST id. + /// Gets AST ID. pub fn id(&self) -> Option { self.ast.id } diff --git a/app/gui/language/ast/impl/src/opr.rs b/app/gui/language/ast/impl/src/opr.rs index 842e09e5b98a..483388e895db 100644 --- a/app/gui/language/ast/impl/src/opr.rs +++ b/app/gui/language/ast/impl/src/opr.rs @@ -260,18 +260,18 @@ impl GeneralizedInfix { } /// The self operand, target of the application. - pub fn target_operand(&self) -> Operand { + pub fn target_operand(&self) -> &Operand { match self.assoc() { - Assoc::Left => self.left.clone(), - Assoc::Right => self.right.clone(), + Assoc::Left => &self.left, + Assoc::Right => &self.right, } } /// Operand other than self. - pub fn argument_operand(&self) -> Operand { + pub fn argument_operand(&self) -> &Operand { match self.assoc() { - Assoc::Left => self.right.clone(), - Assoc::Right => self.left.clone(), + Assoc::Left => &self.right, + Assoc::Right => &self.left, } } @@ -283,11 +283,11 @@ impl GeneralizedInfix { } fn flatten_with_offset(&self, offset: usize) -> Chain { - let target = self.target_operand(); + let target = self.target_operand().clone(); let rest = ChainElement { offset, operator: self.opr.clone(), - operand: self.argument_operand(), + operand: self.argument_operand().clone(), infix_id: self.id, }; diff --git a/app/gui/language/span-tree/example/src/lib.rs b/app/gui/language/span-tree/example/src/lib.rs index 4cb2c87551ca..4f6b16750073 100644 --- a/app/gui/language/span-tree/example/src/lib.rs +++ b/app/gui/language/span-tree/example/src/lib.rs @@ -28,7 +28,7 @@ pub fn main() { let parens_cr = ast::crumbs::MatchCrumb::Segs { val, index: 0 }; let _input_span_tree = builder::TreeBuilder::<()>::new(36) .add_child(0, 14, node::Kind::Chained, PrefixCrumb::Func) - .add_child(0, 9, node::Kind::Operation, PrefixCrumb::Func) + .add_child(0, 9, node::Kind::operation(), PrefixCrumb::Func) .set_ast_id(Uuid::new_v4()) .done() .add_empty_child(10, InsertionPointType::BeforeTarget) @@ -42,7 +42,7 @@ pub fn main() { .set_ast_id(Uuid::new_v4()) .add_child(1, 19, node::Kind::argument(), parens_cr1) .set_ast_id(Uuid::new_v4()) - .add_child(0, 12, node::Kind::Operation, PrefixCrumb::Func) + .add_child(0, 12, node::Kind::operation(), PrefixCrumb::Func) .set_ast_id(Uuid::new_v4()) .done() .add_empty_child(13, InsertionPointType::BeforeTarget) @@ -62,7 +62,7 @@ pub fn main() { .crumbs(PrefixCrumb::Func) .new_child(|t| { t.size(9.bytes()) - .kind(node::Kind::Operation) + .kind(node::Kind::operation()) .crumbs(PrefixCrumb::Func) .new_ast_id() }) @@ -86,7 +86,7 @@ pub fn main() { .crumbs(parens_cr) .new_child(|t| { t.size(12.bytes()) - .kind(node::Kind::Operation) + .kind(node::Kind::operation()) .crumbs(PrefixCrumb::Func) .new_ast_id() }) diff --git a/app/gui/language/span-tree/src/action.rs b/app/gui/language/span-tree/src/action.rs index 6746fde15943..439b22b7c9ba 100644 --- a/app/gui/language/span-tree/src/action.rs +++ b/app/gui/language/span-tree/src/action.rs @@ -118,7 +118,7 @@ impl<'a, T> Implementation for node::Ref<'a, T> { use node::InsertionPointType::*; let kind = &ins_point.kind; let ast = root.get_traversing(&self.ast_crumbs)?; - let expect_arg = matches!(kind, ExpectedArgument(_)); + let expect_arg = kind.is_expected_argument(); let extended_infix = (!expect_arg).and_option_from(|| ast::opr::Chain::try_new(ast)); let new_ast = modify_preserving_id(ast, |ast| { @@ -256,7 +256,7 @@ mod test { fn run(&self, parser: &Parser) { let ast = parser.parse_line_ast(self.expr).unwrap(); let ast_id = ast.id; - let tree = ast.generate_tree(&context::Empty).unwrap(): SpanTree; + let tree: SpanTree = ast.generate_tree(&context::Empty).unwrap(); let node = tree.root_ref().find_by_span(&self.span.clone().into()); let node = node.unwrap_or_else(|| { panic!("Invalid case {:?}: no node with span {:?}", self, self.span) @@ -269,7 +269,7 @@ mod test { .unwrap(); let result_repr = result.repr(); assert_eq!(result_repr, self.expected, "Wrong answer for case {self:?}"); - assert_eq!(ast_id, result.id, "Changed AST id in case {self:?}"); + assert_eq!(ast_id, result.id, "Changed AST ID in case {self:?}"); } } @@ -401,7 +401,7 @@ mod test { // Consider Span Tree for `foo bar` where `foo` is a method known to take 3 parameters. // We can try setting each of 3 arguments to `baz`. let tree = TreeBuilder::<()>::new(7) - .add_leaf(0, 3, node::Kind::Operation, PrefixCrumb::Func) + .add_leaf(0, 3, node::Kind::operation(), PrefixCrumb::Func) .add_leaf(4, 7, node::Kind::this(), PrefixCrumb::Arg) .add_empty_child(7, ExpectedArgument(1)) .add_empty_child(7, ExpectedArgument(2)) @@ -427,7 +427,7 @@ mod test { // parameters. We can try setting each of 2 arguments to `baz`. let tree: SpanTree = TreeBuilder::new(10) .add_leaf(0, 4, node::Kind::this(), InfixCrumb::LeftOperand) - .add_leaf(5, 6, node::Kind::Operation, InfixCrumb::Operator) + .add_leaf(5, 6, node::Kind::operation(), InfixCrumb::Operator) .add_leaf(7, 10, node::Kind::argument(), InfixCrumb::RightOperand) .add_empty_child(10, ExpectedArgument(0)) .add_empty_child(10, ExpectedArgument(1)) diff --git a/app/gui/language/span-tree/src/generate.rs b/app/gui/language/span-tree/src/generate.rs index a1841b24be8a..2364c0d5f58d 100644 --- a/app/gui/language/span-tree/src/generate.rs +++ b/app/gui/language/span-tree/src/generate.rs @@ -18,6 +18,7 @@ use ast::Ast; use ast::HasRepr; use ast::MacroAmbiguousSegment; use ast::MacroMatchSegment; +use std::collections::VecDeque; // ============== @@ -150,34 +151,77 @@ impl ChildGenerator { #[derive(Clone, Debug)] struct ApplicationBase<'a> { /// The name of invoked function. - function_name: Option<&'a str>, - /// True when Ast uses method notation to pass this as an invocation target. - has_target: bool, + function_name: Option>, + /// True when Ast uses method notation to pass `self` as an invocation target. + uses_method_notation: bool, + /// AST ID of the function application expression. The subject of [`Context::call_info`] call, + /// which is used to get a list of expected arguments for this application. + /// + /// For an expression `target.method arg`, this is the `target.method` part. + call_id: Option, } impl<'a> ApplicationBase<'a> { - fn new(ast: &'a Ast) -> Self { - if let Some(chain) = ast::opr::as_access_chain(ast) { - let get_name = || -> Option<&'a str> { - let crumbs = chain.enumerate_operands().last()??.crumbs; - let ast = ast.get_traversing(&crumbs).ok()?; - ast::identifier::name(ast) - }; - ApplicationBase { function_name: get_name(), has_target: true } + /// Create `ApplicationBase` from infix expression. + fn from_infix(infix: &'a GeneralizedInfix) -> Self { + let call_id = infix.id; + if infix.name() == ast::opr::predefined::ACCESS { + // method-style notation: `this.that.method` + // In this case the applied method is not the dot operator, but the name after the dot. + let function = infix.argument_operand().as_ref(); + let function_name = + function.and_then(|func| ast::identifier::name(&func.arg)).map(Into::into); + ApplicationBase { function_name, call_id, uses_method_notation: true } } else { - ApplicationBase { function_name: ast::identifier::name(ast), has_target: false } + // Other chain type, e.g. `a + b + c` + let function_name = Some(infix.name().into()); + ApplicationBase { function_name, call_id, uses_method_notation: false } } } - fn prefix_params( - &self, - invocation_info: Option, - ) -> impl ExactSizeIterator { - let mut ret = invocation_info.map(|info| info.parameters).unwrap_or_default().into_iter(); - if self.has_target { - ret.next(); + /// Create `ApplicationBase` from flattened prefix expression chain. + fn from_prefix_chain(chain: &'a ast::prefix::Chain) -> Self { + let call_id = chain.id(); + // When first chain element is an infix access, derive the application base from it, but + // treat the whole chain as a call expression. + if let Some(access_infix) = GeneralizedInfix::try_new(&chain.func) + .filter(|infix| infix.name() == ast::opr::predefined::ACCESS) + { + let base = ApplicationBase::from_infix(&access_infix).into_owned(); + return ApplicationBase { call_id, ..base }; } - ret + + // For any other prefix chain, the applied function is the first chain element. + let function_name = ast::identifier::name(&chain.func).map(Into::into); + ApplicationBase { function_name, call_id, uses_method_notation: false } + } + + /// Get the list of method prefix arguments expected by this application. Does not include + /// `self` parameter when using method notation. Returns `None` when the call info is not + /// present in the context or AST doesn't contain enough information to query it. + fn known_prefix_arguments(&self, context: &impl Context) -> Option> { + // If method notation is used, the function name is required to get relevant call info. Do + // not attempt the call if method name is not available, as the returned data would be not + // relevant. + if self.uses_method_notation && self.function_name.is_none() { + return None; + } + + let invocation_info = context.call_info(self.call_id?, self.function_name.as_deref())?; + let parameters = invocation_info.with_call_id(self.call_id).parameters; + let mut deque: VecDeque = parameters.into(); + + // When a method notation is used, the first received argument is the target. Remove it from + // the list of expected prefix arguments. + if self.uses_method_notation { + deque.pop_front(); + } + Some(deque) + } + + fn into_owned(self) -> ApplicationBase<'static> { + let function_name = self.function_name.map(|s| s.into_owned().into()); + ApplicationBase { function_name, ..self } } } @@ -199,21 +243,31 @@ fn generate_node_for_ast( kind: node::Kind, context: &impl Context, ) -> FallibleResult> { - // Code like `ast.func` or `a+b+c`. if let Some(infix) = GeneralizedInfix::try_new(ast) { + // Code like `ast.func` or `a+b+c`. + let app_base = ApplicationBase::from_infix(&infix); let chain = infix.flatten(); - let app_base = ApplicationBase::new(ast); - let invocation = || -> Option { - context.call_info(ast.id?, Some(app_base.function_name?)) - }(); - - // All prefix params are missing arguments, since there is no prefix application. - let missing_args = app_base.prefix_params(invocation); - let arity = missing_args.len(); - let base_node_kind = if arity == 0 { kind.clone() } else { node::Kind::Operation }; - let node = chain.generate_node(base_node_kind, context)?; - let provided_prefix_arg_count = 0; - Ok(generate_expected_arguments(node, kind, provided_prefix_arg_count, missing_args)) + + if app_base.uses_method_notation { + // For method call, this is behaving like a prefix with single member. All prefix params + // are missing arguments, since there is no prefix application. + + let missing_args = app_base.known_prefix_arguments(context).unwrap_or_default(); + let arity = missing_args.len(); + let base_node_kind = if arity == 0 { + kind.clone() + } else { + node::Kind::operation().with_call_id(app_base.call_id).into() + }; + + let node = chain.generate_node(base_node_kind, context)?; + let provided_prefix_arg_count = 0; + let args_iter = missing_args.into_iter(); + Ok(generate_expected_arguments(node, kind, provided_prefix_arg_count, args_iter)) + } else { + // For non-access infix operators, missing arguments are not handled at this level. + chain.generate_node(kind, context) + } } else { match ast.shape() { ast::Shape::Prefix(_) => @@ -233,13 +287,14 @@ fn generate_node_for_ast( let payload = default(); if let Some(info) = ast.id.and_then(|id| context.call_info(id, name)) { let node = { - let kind = node::Kind::Operation; + let kind = node::Kind::operation().with_call_id(ast.id).into(); Node { kind, size, children, ast_id, payload } }; // Note that in this place it is impossible that Ast is in form of // `this.method` -- it is covered by the former if arm. As such, we don't // need to use `ApplicationBase` here as we do elsewhere. let provided_prefix_arg_count = 0; + let info = info.with_call_id(ast.id); let params = info.parameters.into_iter(); Ok(generate_expected_arguments(node, kind, provided_prefix_arg_count, params)) } else { @@ -268,12 +323,16 @@ fn generate_node_for_opr_chain( kind: node::Kind, context: &impl Context, ) -> FallibleResult> { + let is_access = this.operator.name == ast::opr::predefined::ACCESS; + let this_call_id = + if is_access { kind.call_id() } else { this.args.first().and_then(|elem| elem.infix_id) }; + // Removing operands is possible only when chain has at least 3 of them // (target and two arguments). let removable = this.args.len() >= 2; let node_and_offset: FallibleResult<(Node, usize)> = match &this.target { Some(target) => { - let kind = node::Kind::this().with_removable(removable); + let kind = node::Kind::this().with_removable(removable).with_call_id(this_call_id); let node = target.arg.generate_node(kind, context)?; Ok((node, target.offset)) } @@ -303,17 +362,15 @@ fn generate_node_for_opr_chain( gen.generate_empty_node(InsertionPointType::AfterTarget); } gen.spacing(off); - gen.generate_ast_node(opr_ast, node::Kind::Operation, context)?; + gen.generate_ast_node(opr_ast, node::Kind::operation(), context)?; if let Some(operand) = &elem.operand { let arg_crumbs = elem.crumb_to_operand(has_left); let arg_ast = Located::new(arg_crumbs, operand.arg.clone_ref()); gen.spacing(operand.offset); - gen.generate_ast_node( - arg_ast, - node::Kind::argument().with_removable(removable), - context, - )?; + let arg_call_id = if is_access { None } else { elem.infix_id }; + let arg = node::Kind::argument().with_removable(removable).with_call_id(arg_call_id); + gen.generate_ast_node(arg_ast, arg, context)?; } gen.generate_empty_node(InsertionPointType::Append); @@ -353,21 +410,24 @@ fn generate_node_for_prefix_chain( kind: node::Kind, context: &impl Context, ) -> FallibleResult> { - let base = ApplicationBase::new(&this.func); - let invocation_info = this.id().and_then(|id| context.call_info(id, base.function_name)); - let known_args = invocation_info.is_some(); - let mut known_params = base.prefix_params(invocation_info); + let app_base = ApplicationBase::from_prefix_chain(this); + let known_params = app_base.known_prefix_arguments(context); + let uses_method_notation = app_base.uses_method_notation; + let known_args = known_params.is_some(); + let mut known_params = known_params.unwrap_or_default(); + let prefix_arity = this.args.len().max(known_params.len()); use ast::crumbs::PrefixCrumb::*; // Removing arguments is possible if there at least two of them let removable = this.args.len() >= 2; - let node = this.func.generate_node(node::Kind::Operation, context); + let node = + this.func.generate_node(node::Kind::operation().with_call_id(app_base.call_id), context); let ret = this.args.iter().enumerate().fold(node, |node, (i, arg)| { let node = node?; let is_first = i == 0; let is_last = i + 1 == prefix_arity; - let arg_kind = if is_first && !base.has_target { + let arg_kind = if is_first && !uses_method_notation { node::Kind::from(node::Kind::this().with_removable(removable)) } else { node::Kind::from(node::Kind::argument().with_removable(removable)) @@ -382,7 +442,7 @@ fn generate_node_for_prefix_chain( let arg_ast = arg.sast.wrapped.clone_ref(); let arg_child: &mut node::Child = gen.generate_ast_node(Located::new(Arg, arg_ast), arg_kind, context)?; - if let Some(info) = known_params.next() { + if let Some(info) = known_params.pop_front() { arg_child.node.set_argument_info(info) } if !known_args { @@ -397,7 +457,7 @@ fn generate_node_for_prefix_chain( }) })?; - Ok(generate_expected_arguments(ret, kind, this.args.len(), known_params)) + Ok(generate_expected_arguments(ret, kind, this.args.len(), known_params.into_iter())) } @@ -637,7 +697,7 @@ mod test { /// `node`. /// /// It is used in tests. Because parser can assign id as he pleases, therefore to keep tests - /// cleaner the expression ids are removed before comparing trees. + /// cleaner the expression IDs are removed before comparing trees. fn clear_expression_ids(node: &mut Node) { node.ast_id = None; for child in &mut node.children { @@ -668,7 +728,7 @@ mod test { let ast = parser.parse_line_ast_with_id_map("2 + foo bar - 3", id_map.clone()).unwrap(); let mut tree: SpanTree = ast.generate_tree(&context::Empty).unwrap(); - // Check the expression ids we defined: + // Check the expression IDs we defined: for id_map_entry in id_map.vec { let (span, id) = id_map_entry; let node = tree.root_ref().find_by_span(&span); @@ -678,15 +738,16 @@ mod test { // Check the other fields: clear_expression_ids(&mut tree.root); + clear_parameter_infos(&mut tree.root); let expected = TreeBuilder::new(15) .add_empty_child(0, BeforeTarget) .add_child(0, 11, node::Kind::this(), InfixCrumb::LeftOperand) .add_empty_child(0, BeforeTarget) .add_leaf(0, 1, node::Kind::this(), InfixCrumb::LeftOperand) .add_empty_child(1, AfterTarget) - .add_leaf(2, 1, node::Kind::Operation, InfixCrumb::Operator) + .add_leaf(2, 1, node::Kind::operation(), InfixCrumb::Operator) .add_child(4, 7, node::Kind::argument(), InfixCrumb::RightOperand) - .add_leaf(0, 3, node::Kind::Operation, PrefixCrumb::Func) + .add_leaf(0, 3, node::Kind::operation(), PrefixCrumb::Func) .add_empty_child(4, BeforeTarget) .add_leaf(4, 3, node::Kind::this(), PrefixCrumb::Arg) .add_empty_child(7, Append) @@ -694,7 +755,7 @@ mod test { .add_empty_child(11, Append) .done() .add_empty_child(11, AfterTarget) - .add_leaf(12, 1, node::Kind::Operation, InfixCrumb::Operator) + .add_leaf(12, 1, node::Kind::operation(), InfixCrumb::Operator) .add_leaf(14, 1, node::Kind::argument(), InfixCrumb::RightOperand) .add_empty_child(15, Append) .build(); @@ -708,6 +769,7 @@ mod test { let ast = parser.parse_line_ast("2 + 3 + foo bar baz 13 + 5").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(26) .add_child(0, 22, node::Kind::Chained, InfixCrumb::LeftOperand) @@ -715,15 +777,15 @@ mod test { .add_empty_child(0, BeforeTarget) .add_leaf(0, 1, node::Kind::this().removable(), InfixCrumb::LeftOperand) .add_empty_child(1, AfterTarget) - .add_leaf(2, 1, node::Kind::Operation, InfixCrumb::Operator) + .add_leaf(2, 1, node::Kind::operation(), InfixCrumb::Operator) .add_leaf(4, 1, node::Kind::argument().removable(), InfixCrumb::RightOperand) .add_empty_child(5, Append) .done() - .add_leaf(6, 1, node::Kind::Operation, InfixCrumb::Operator) + .add_leaf(6, 1, node::Kind::operation(), InfixCrumb::Operator) .add_child(8, 14, node::Kind::argument().removable(), InfixCrumb::RightOperand) .add_child(0, 11, node::Kind::Chained, PrefixCrumb::Func) .add_child(0, 7, node::Kind::Chained, PrefixCrumb::Func) - .add_leaf(0, 3, node::Kind::Operation, PrefixCrumb::Func) + .add_leaf(0, 3, node::Kind::operation(), PrefixCrumb::Func) .add_empty_child(4, BeforeTarget) .add_leaf(4, 3, node::Kind::this().removable(), PrefixCrumb::Arg) .add_empty_child(7, Append) @@ -736,7 +798,7 @@ mod test { .done() .add_empty_child(22, Append) .done() - .add_leaf(23, 1, node::Kind::Operation, InfixCrumb::Operator) + .add_leaf(23, 1, node::Kind::operation(), InfixCrumb::Operator) .add_leaf(25, 1, node::Kind::argument().removable(), InfixCrumb::RightOperand) .add_empty_child(26, Append) .build(); @@ -750,15 +812,16 @@ mod test { let ast = parser.parse_line_ast("1,2,3").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(5) .add_empty_child(0, Append) .add_leaf(0, 1, node::Kind::argument().removable(), InfixCrumb::LeftOperand) - .add_leaf(1, 1, node::Kind::Operation, InfixCrumb::Operator) + .add_leaf(1, 1, node::Kind::operation(), InfixCrumb::Operator) .add_child(2, 3, node::Kind::Chained, InfixCrumb::RightOperand) .add_empty_child(0, Append) .add_leaf(0, 1, node::Kind::argument().removable(), InfixCrumb::LeftOperand) - .add_leaf(1, 1, node::Kind::Operation, InfixCrumb::Operator) + .add_leaf(1, 1, node::Kind::operation(), InfixCrumb::Operator) .add_empty_child(2, AfterTarget) .add_leaf(2, 1, node::Kind::this().removable(), InfixCrumb::RightOperand) .add_empty_child(3, BeforeTarget) @@ -776,28 +839,29 @@ mod test { let ast = parser.parse_line_ast("+ * + + 2 +").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(11) .add_child(0, 9, node::Kind::Chained, SectionLeftCrumb::Arg) .add_child(0, 5, node::Kind::Chained, InfixCrumb::LeftOperand) .add_child(0, 3, node::Kind::Chained, SectionLeftCrumb::Arg) .add_empty_child(0, BeforeTarget) - .add_leaf(0, 1, node::Kind::Operation, SectionRightCrumb::Opr) + .add_leaf(0, 1, node::Kind::operation(), SectionRightCrumb::Opr) .add_child(2, 1, node::Kind::argument().removable(), SectionRightCrumb::Arg) .add_empty_child(0, BeforeTarget) - .add_leaf(0, 1, node::Kind::Operation, SectionSidesCrumb) + .add_leaf(0, 1, node::Kind::operation(), SectionSidesCrumb) .add_empty_child(1, Append) .done() .add_empty_child(3, Append) .done() - .add_leaf(4, 1, node::Kind::Operation, SectionLeftCrumb::Opr) + .add_leaf(4, 1, node::Kind::operation(), SectionLeftCrumb::Opr) .add_empty_child(5, Append) .done() - .add_leaf(6, 1, node::Kind::Operation, InfixCrumb::Operator) + .add_leaf(6, 1, node::Kind::operation(), InfixCrumb::Operator) .add_leaf(8, 1, node::Kind::argument().removable(), InfixCrumb::RightOperand) .add_empty_child(9, Append) .done() - .add_leaf(10, 1, node::Kind::Operation, SectionLeftCrumb::Opr) + .add_leaf(10, 1, node::Kind::operation(), SectionLeftCrumb::Opr) .add_empty_child(11, Append) .build(); @@ -810,14 +874,15 @@ mod test { let ast = parser.parse_line_ast(",2,").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(3) .add_empty_child(0, Append) - .add_leaf(0, 1, node::Kind::Operation, SectionRightCrumb::Opr) + .add_leaf(0, 1, node::Kind::operation(), SectionRightCrumb::Opr) .add_child(1, 2, node::Kind::Chained, SectionRightCrumb::Arg) .add_empty_child(0, Append) .add_leaf(0, 1, node::Kind::argument().removable(), SectionLeftCrumb::Arg) - .add_leaf(1, 1, node::Kind::Operation, SectionLeftCrumb::Opr) + .add_leaf(1, 1, node::Kind::operation(), SectionLeftCrumb::Opr) .add_empty_child(2, BeforeTarget) .done() .build(); @@ -831,17 +896,17 @@ mod test { let parser = Parser::new_or_panic(); let mut id_map = IdMap::default(); - id_map.generate(0..29); + let expected_id = id_map.generate(0..29); let expression = "if foo then (a + b) x else ()"; - let ast = parser.parse_line_ast_with_id_map(expression, id_map.clone()).unwrap(); + let ast = parser.parse_line_ast_with_id_map(expression, id_map).unwrap(); let mut tree: SpanTree = ast.generate_tree(&context::Empty).unwrap(); // Check if expression id is set - let (_, expected_id) = id_map.vec.first().unwrap(); - assert_eq!(tree.root_ref().ast_id, Some(*expected_id)); + assert_eq!(tree.root_ref().ast_id, Some(expected_id)); // Check the other fields clear_expression_ids(&mut tree.root); + clear_parameter_infos(&mut tree.root); let seq = Seq { right: false }; let if_then_else_cr = vec![seq, Or, Build]; let parens_cr = vec![seq, Or, Or, Build]; @@ -851,13 +916,13 @@ mod test { .add_leaf(3, 3, node::Kind::argument(), segment_body_crumbs(0, &if_then_else_cr)) .add_leaf(7, 4, node::Kind::Token, segment_head_crumbs(1)) .add_child(12, 9, node::Kind::argument(), segment_body_crumbs(1, &if_then_else_cr)) - .add_child(0, 7, node::Kind::Operation, PrefixCrumb::Func) + .add_child(0, 7, node::Kind::operation(), PrefixCrumb::Func) .add_leaf(0, 1, node::Kind::Token, segment_head_crumbs(0)) .add_child(1, 5, node::Kind::argument(), segment_body_crumbs(0, &parens_cr)) .add_empty_child(0, BeforeTarget) .add_leaf(0, 1, node::Kind::this(), InfixCrumb::LeftOperand) .add_empty_child(1, AfterTarget) - .add_leaf(2, 1, node::Kind::Operation, InfixCrumb::Operator) + .add_leaf(2, 1, node::Kind::operation(), InfixCrumb::Operator) .add_leaf(4, 1, node::Kind::argument(), InfixCrumb::RightOperand) .add_empty_child(5, Append) .done() @@ -936,9 +1001,10 @@ mod test { 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); + clear_parameter_infos(&mut tree.root); let expected = TreeBuilder::new(13) - .add_leaf(0, 3, node::Kind::Operation, PrefixCrumb::Func) + .add_leaf(0, 3, node::Kind::operation(), PrefixCrumb::Func) .add_empty_child(4, BeforeTarget) .add_leaf(4, 9, node::Kind::this(), PrefixCrumb::Arg) .add_empty_child(13, Append) @@ -950,28 +1016,40 @@ mod test { #[wasm_bindgen_test] fn generating_span_tree_for_unfinished_call() { let parser = Parser::new_or_panic(); - let this_param = - ArgumentInfo { name: Some("self".to_owned()), tp: Some("Any".to_owned()), ..default() }; - let param1 = ArgumentInfo { + let this_param = |call_id| ArgumentInfo { + name: Some("self".to_owned()), + tp: Some("Any".to_owned()), + call_id, + ..default() + }; + let param1 = |call_id| ArgumentInfo { name: Some("arg1".to_owned()), tp: Some("Number".to_owned()), + call_id, + ..default() + }; + let param2 = |call_id| ArgumentInfo { + name: Some("arg2".to_owned()), + tp: None, + call_id, ..default() }; - let param2 = ArgumentInfo { name: Some("arg2".to_owned()), tp: None, ..default() }; // === Single function name === - let ast = parser.parse_line_ast("foo").unwrap(); - let invocation_info = CalledMethodInfo { parameters: vec![this_param.clone()] }; + let mut id_map = IdMap::default(); + let call_id = id_map.generate(0..3); + let ast = parser.parse_line_ast_with_id_map("foo", id_map).unwrap(); + let invocation_info = CalledMethodInfo { parameters: vec![this_param(None)] }; let ctx = MockContext::new_single(ast.id.unwrap(), invocation_info); let mut tree: SpanTree = SpanTree::new(&ast, &ctx).unwrap(); match tree.root_ref().leaf_iter().collect_vec().as_slice() { - [_func, arg0] => assert_eq!(arg0.argument_info().as_ref(), Some(&this_param)), + [_func, arg0] => assert_eq!(arg0.argument_info(), Some(this_param(Some(call_id)))), sth_else => panic!("There should be 2 leaves, found: {}", sth_else.len()), } let expected = TreeBuilder::new(3) - .add_leaf(0, 3, node::Kind::Operation, Crumbs::default()) + .add_leaf(0, 3, node::Kind::operation(), Crumbs::default()) .add_empty_child(3, ExpectedArgument(0)) .build(); clear_expression_ids(&mut tree.root); @@ -981,16 +1059,18 @@ mod test { // === Complete application chain === - let ast = parser.parse_line_ast("foo here").unwrap(); - let invocation_info = CalledMethodInfo { parameters: vec![this_param.clone()] }; + let mut id_map = IdMap::default(); + let call_id = id_map.generate(0..8); + let ast = parser.parse_line_ast_with_id_map("foo here", id_map).unwrap(); + let invocation_info = CalledMethodInfo { parameters: vec![this_param(None)] }; let ctx = MockContext::new_single(ast.id.unwrap(), invocation_info); let mut tree: SpanTree = SpanTree::new(&ast, &ctx).unwrap(); match tree.root_ref().leaf_iter().collect_vec().as_slice() { - [_func, arg0] => assert_eq!(arg0.argument_info().as_ref(), Some(&this_param)), + [_func, arg0] => assert_eq!(arg0.argument_info(), Some(this_param(Some(call_id)))), sth_else => panic!("There should be 2 leaves, found: {}", sth_else.len()), } let expected = TreeBuilder::new(8) - .add_leaf(0, 3, node::Kind::Operation, PrefixCrumb::Func) + .add_leaf(0, 3, node::Kind::operation(), PrefixCrumb::Func) .add_leaf(4, 4, node::Kind::this(), PrefixCrumb::Arg) .build(); clear_expression_ids(&mut tree.root); @@ -1000,24 +1080,25 @@ mod test { // === Partial application chain === - let ast = parser.parse_line_ast("foo here").unwrap(); - let invocation_info = CalledMethodInfo { - parameters: vec![this_param.clone(), param1.clone(), param2.clone()], - }; + let mut id_map = IdMap::default(); + let call_id = Some(id_map.generate(0..8)); + let ast = parser.parse_line_ast_with_id_map("foo here", id_map).unwrap(); + let invocation_info = + CalledMethodInfo { parameters: vec![this_param(None), param1(None), param2(None)] }; let ctx = MockContext::new_single(ast.id.unwrap(), invocation_info); let mut tree: SpanTree = SpanTree::new(&ast, &ctx).unwrap(); match tree.root_ref().leaf_iter().collect_vec().as_slice() { [_func, arg0, arg1, arg2] => { - assert_eq!(arg0.argument_info().as_ref(), Some(&this_param)); - assert_eq!(arg1.argument_info().as_ref(), Some(¶m1)); - assert_eq!(arg2.argument_info().as_ref(), Some(¶m2)); + assert_eq!(arg0.argument_info(), Some(this_param(call_id))); + assert_eq!(arg1.argument_info(), Some(param1(call_id))); + assert_eq!(arg2.argument_info(), Some(param2(call_id))); } sth_else => panic!("There should be 4 leaves, found: {}", sth_else.len()), } let expected = TreeBuilder::new(8) .add_child(0, 8, node::Kind::Chained, Crumbs::default()) .add_child(0, 8, node::Kind::Chained, Crumbs::default()) - .add_leaf(0, 3, node::Kind::Operation, PrefixCrumb::Func) + .add_leaf(0, 3, node::Kind::operation(), PrefixCrumb::Func) .add_leaf(4, 4, node::Kind::this(), PrefixCrumb::Arg) .done() .add_empty_child(8, ExpectedArgument(1)) @@ -1030,26 +1111,27 @@ mod test { // === Partial application chain - this argument === - - let ast = parser.parse_line_ast("here.foo").unwrap(); + let mut id_map = IdMap::default(); + let call_id = Some(id_map.generate(0..8)); + let ast = parser.parse_line_ast_with_id_map("here.foo", id_map).unwrap(); let invocation_info = - CalledMethodInfo { parameters: vec![this_param, param1.clone(), param2.clone()] }; + CalledMethodInfo { parameters: vec![this_param(None), param1(None), param2(None)] }; let ctx = MockContext::new_single(ast.id.unwrap(), invocation_info); let mut tree: SpanTree = SpanTree::new(&ast, &ctx).unwrap(); match tree.root_ref().leaf_iter().collect_vec().as_slice() { [_, _this, _, _, _func, _, arg1, arg2] => { - assert_eq!(arg1.argument_info().as_ref(), Some(¶m1)); - assert_eq!(arg2.argument_info().as_ref(), Some(¶m2)); + assert_eq!(arg1.argument_info(), Some(param1(call_id))); + assert_eq!(arg2.argument_info(), Some(param2(call_id))); } sth_else => panic!("There should be 8 leaves, found: {}", sth_else.len()), } let expected = TreeBuilder::new(8) .add_child(0, 8, node::Kind::Chained, Crumbs::default()) - .add_child(0, 8, node::Kind::Operation, Crumbs::default()) + .add_child(0, 8, node::Kind::operation(), Crumbs::default()) .add_empty_child(0, BeforeTarget) .add_leaf(0, 4, node::Kind::this(), InfixCrumb::LeftOperand) .add_empty_child(4, AfterTarget) - .add_leaf(4, 1, node::Kind::Operation, InfixCrumb::Operator) + .add_leaf(4, 1, node::Kind::operation(), InfixCrumb::Operator) .add_leaf(5, 3, node::Kind::argument(), InfixCrumb::RightOperand) .add_empty_child(8, Append) .done() diff --git a/app/gui/language/span-tree/src/generate/context.rs b/app/gui/language/span-tree/src/generate/context.rs index b35418df606c..af4d17be1d44 100644 --- a/app/gui/language/span-tree/src/generate/context.rs +++ b/app/gui/language/span-tree/src/generate/context.rs @@ -17,6 +17,16 @@ pub struct CalledMethodInfo { pub parameters: Vec, } +impl CalledMethodInfo { + /// Assign call and target expression IDs to all parameters. + pub fn with_call_id(mut self, call_id: Option) -> Self { + self.parameters.iter_mut().for_each(|arg| { + arg.call_id = call_id; + }); + self + } +} + // =============== diff --git a/app/gui/language/span-tree/src/iter.rs b/app/gui/language/span-tree/src/iter.rs index d6757f032a88..c50c398f978a 100644 --- a/app/gui/language/span-tree/src/iter.rs +++ b/app/gui/language/span-tree/src/iter.rs @@ -133,19 +133,19 @@ mod tests { let tree: SpanTree = TreeBuilder::new(14) .add_child(0, 10, Chained, vec![LeftOperand]) .add_leaf(0, 3, Kind::this(), vec![LeftOperand]) - .add_leaf(4, 1, Operation, vec![Operator]) + .add_leaf(4, 1, Kind::operation(), vec![Operator]) .add_child(6, 3, Kind::argument(), vec![RightOperand]) - .add_leaf(0, 1, Operation, vec![Func]) + .add_leaf(0, 1, Kind::operation(), vec![Func]) .add_leaf(2, 1, Kind::this(), vec![Arg]) .done() .done() - .add_leaf(11, 1, Operation, vec![Operator]) + .add_leaf(11, 1, Kind::operation(), vec![Operator]) .add_child(13, 1, Chained, vec![RightOperand]) .add_leaf(0, 3, Kind::this(), vec![LeftOperand]) - .add_leaf(4, 1, Operation, vec![Operator]) + .add_leaf(4, 1, Kind::operation(), vec![Operator]) .add_child(6, 5, Chained, vec![RightOperand]) .add_leaf(0, 1, Kind::this(), vec![LeftOperand]) - .add_leaf(2, 1, Operation, vec![Operator]) + .add_leaf(2, 1, Kind::operation(), vec![Operator]) .add_leaf(4, 1, Kind::argument(), vec![RightOperand]) .done() .done() diff --git a/app/gui/language/span-tree/src/lib.rs b/app/gui/language/span-tree/src/lib.rs index da9892e5c490..a0b24e8f8905 100644 --- a/app/gui/language/span-tree/src/lib.rs +++ b/app/gui/language/span-tree/src/lib.rs @@ -77,19 +77,32 @@ use crate::generate::Context; pub struct ArgumentInfo { pub name: Option, pub tp: Option, + /// The AST ID of the call expression that this argument is passed to. + /// See [`ApplicationBase`] for more details. + pub call_id: Option, pub tag_values: Vec, } impl ArgumentInfo { /// Constructor. - pub fn new(name: Option, tp: Option, tag_values: Vec) -> Self { - Self { name, tp, tag_values } + pub fn new( + name: Option, + tp: Option, + call_id: Option, + tag_values: Vec, + ) -> Self { + Self { name, tp, call_id, tag_values } } /// Specialized constructor for "this" argument. - pub fn this(tp: Option) -> Self { + pub fn this(tp: Option, call_id: Option) -> Self { let name = Some(node::This::NAME.into()); - Self { name, tp, tag_values: Vec::new() } + Self { name, tp, call_id, tag_values: Vec::new() } + } + + /// Extend the argument info with the given call id. + pub fn with_call_id(self, call_id: Option) -> Self { + Self { call_id, ..self } } } @@ -169,3 +182,102 @@ impl Default for SpanTree { Self { root } } } + +// == Debug utils == + +impl SpanTree { + #[allow(dead_code)] + /// Get pretty-printed representation of this span tree for debugging purposes. The `code` + /// argument should be identical to the expression that was used during generation of this + /// span-tree. It will be used to print code fragments associated with each span. + /// + /// Example output with AST ids removed for clarity: + /// ```text + /// operator6.join operator31 Join_Kind.Inner ["County"] Root + /// operator6.join operator31 Join_Kind.Inner ["County"] ├── Chained + /// operator6.join operator31 Join_Kind.Inner ["County"] │ ├── Chained + /// operator6.join operator31 Join_Kind.Inner │ │ ├── Chained + /// operator6.join operator31 │ │ │ ├── Chained + /// operator6.join │ │ │ │ ├── Operation + /// ▲ │ │ │ │ │ ├── InsertionPoint(BeforeTarget) + /// operator6 │ │ │ │ │ ├── This + /// ▲ │ │ │ │ │ ├── InsertionPoint(AfterTarget) + /// . │ │ │ │ │ ├── Operation + /// join │ │ │ │ │ ├── Argument + /// ▲ │ │ │ │ │ ╰── InsertionPoint(Append) + /// operator31 │ │ │ │ ╰── Argument name="right" + /// Join_Kind.Inner │ │ │ ╰── Argument name="join_kind" + /// ▲ │ │ │ ├── InsertionPoint(BeforeTarget) + /// Join_Kind │ │ │ ├── This + /// ▲ │ │ │ ├── InsertionPoint(AfterTarget) + /// . │ │ │ ├── Operation + /// Inner │ │ │ ├── Argument + /// ▲ │ │ │ ╰── InsertionPoint(Append) + /// ["County"] │ │ ╰── Argument name="on" + /// [ │ │ ├── Token + /// "County" │ │ ├── Argument + /// ] │ │ ╰── Token + /// ▲│ ╰── InsertionPoint(ExpectedArgument(3)) name="right_prefix" + /// ▲╰── InsertionPoint(ExpectedArgument(4)) name="on_problems" + /// ``` + pub fn debug_print(&self, code: &str) -> String { + use std::fmt::Write; + + let mut buffer = String::new(); + let span_padding = " ".repeat(code.len() + 1); + + struct PrintState { + indent: String, + num_children: usize, + } + let state = PrintState { indent: String::new(), num_children: 1 }; + self.root_ref().dfs_with_layer_data(state, |node, state| { + let span = node.span(); + let node_code = &code[span]; + buffer.push_str(&span_padding[0..node.span_offset.into()]); + let mut written = node.span_offset.into(); + if node_code.is_empty() { + buffer.push('▲'); + written += 1; + } else { + buffer.push_str(node_code); + written += node_code.len(); + } + buffer.push_str(&span_padding[written..]); + + let indent = if let Some(index) = node.crumbs.last() { + let is_last = *index == state.num_children - 1; + let indent_targeted = if is_last { "╰── " } else { "├── " }; + let indent_continue = if is_last { " " } else { "│ " }; + + buffer.push_str(&state.indent); + buffer.push_str(indent_targeted); + format!("{}{}", state.indent, indent_continue) + } else { + state.indent.clone() + }; + + buffer.push_str(node.kind.variant_name()); + if let node::Kind::InsertionPoint(inner) = &node.kind { + write!(buffer, "({:?})", inner.kind).unwrap(); + } + + if let Some(name) = node.kind.name() { + write!(buffer, " name={name:?}").unwrap(); + } + + if let Some(call_id) = node.kind.call_id() { + write!(buffer, " call_id={call_id:?}").unwrap(); + } + + if let Some(ast_id) = node.ast_id { + write!(buffer, " ast_id={ast_id:?}").unwrap(); + } + buffer.push('\n'); + + let num_children = node.children.len(); + PrintState { indent, num_children } + }); + buffer + } +} diff --git a/app/gui/language/span-tree/src/node.rs b/app/gui/language/span-tree/src/node.rs index 3d8625f7de9c..1490ea5c3c58 100644 --- a/app/gui/language/span-tree/src/node.rs +++ b/app/gui/language/span-tree/src/node.rs @@ -122,6 +122,9 @@ impl Node { pub fn is_expected_argument(&self) -> bool { self.kind.is_expected_argument() } + pub fn is_function_parameter(&self) -> bool { + self.kind.is_function_parameter() + } } @@ -742,6 +745,12 @@ impl<'a, T> Deref for RefMut<'a, T> { } } +impl<'a, T> DerefMut for RefMut<'a, T> { + fn deref_mut(&mut self) -> &mut Self::Target { + self.node + } +} + // === Specialized Iterators === @@ -859,10 +868,10 @@ mod test { let tree: SpanTree = TreeBuilder::new(7) .add_leaf(0, 1, node::Kind::this(), vec![LeftOperand]) - .add_leaf(1, 1, node::Kind::Operation, vec![Operator]) + .add_leaf(1, 1, node::Kind::operation(), vec![Operator]) .add_child(2, 5, node::Kind::argument(), vec![RightOperand]) .add_leaf(0, 2, node::Kind::this(), vec![LeftOperand]) - .add_leaf(3, 1, node::Kind::Operation, vec![Operator]) + .add_leaf(3, 1, node::Kind::operation(), vec![Operator]) .add_leaf(4, 1, node::Kind::argument(), vec![RightOperand]) .done() .build(); @@ -918,9 +927,9 @@ mod test { let tree: SpanTree = TreeBuilder::new(7) .add_leaf(0, 1, node::Kind::this(), vec![LeftOperand]) .add_empty_child(1, InsertionPointType::AfterTarget) - .add_leaf(1, 1, node::Kind::Operation, vec![Operator]) + .add_leaf(1, 1, node::Kind::operation(), vec![Operator]) .add_child(2, 5, node::Kind::argument(), vec![RightOperand]) - .add_leaf(0, 3, node::Kind::Operation, vec![Func]) + .add_leaf(0, 3, node::Kind::operation(), vec![Func]) .add_leaf(3, 1, node::Kind::this(), vec![Arg]) .done() .build(); @@ -950,9 +959,9 @@ mod test { // See also `generate::test::generating_span_tree_for_unfinished_call` let tree: SpanTree = TreeBuilder::new(8) .add_child(0, 8, node::Kind::Chained, ast::crumbs::Crumbs::default()) - .add_child(0, 8, node::Kind::Operation, ast::crumbs::Crumbs::default()) + .add_child(0, 8, node::Kind::operation(), ast::crumbs::Crumbs::default()) .add_leaf(0, 4, node::Kind::this(), LeftOperand) - .add_leaf(4, 1, node::Kind::Operation, Operator) + .add_leaf(4, 1, node::Kind::operation(), Operator) .add_leaf(5, 3, node::Kind::argument(), RightOperand) .done() .add_empty_child(8, InsertionPointType::ExpectedArgument(0)) diff --git a/app/gui/language/span-tree/src/node/kind.rs b/app/gui/language/span-tree/src/node/kind.rs index 44d74c14aa5b..07126c71ee30 100644 --- a/app/gui/language/span-tree/src/node/kind.rs +++ b/app/gui/language/span-tree/src/node/kind.rs @@ -19,7 +19,7 @@ pub enum Kind { /// A node chained with parent node. See crate's docs for more info about chaining. Chained, /// A node representing operation (operator or function) of parent Infix, Section or Prefix. - Operation, + Operation(Operation), /// A node being a target (or "self") parameter of parent Infix, Section or Prefix. This(This), /// A node being a normal (not target) parameter of parent Infix, Section or Prefix. @@ -38,6 +38,9 @@ pub enum Kind { #[allow(missing_docs)] impl Kind { + pub fn operation() -> Operation { + default() + } pub fn this() -> This { default() } @@ -89,6 +92,12 @@ impl Kind { _ => false, } } + + /// Match any kind that can be a function parameter. This includes `This`, `Argument` and + /// expected argument. + pub fn is_function_parameter(&self) -> bool { + self.is_this() || self.is_argument() || self.is_expected_argument() + } } @@ -127,32 +136,50 @@ impl Kind { /// information. pub fn argument_info(&self) -> Option { Some(match self { - Self::This(t) => ArgumentInfo::this(t.tp.clone()), + Self::This(t) => ArgumentInfo::this(t.tp.clone(), t.call_id), Self::Argument(t) => - ArgumentInfo::new(t.name.clone(), t.tp.clone(), t.tag_values.clone()), + ArgumentInfo::new(t.name.clone(), t.tp.clone(), t.call_id, t.tag_values.clone()), Self::InsertionPoint(t) => - ArgumentInfo::new(t.name.clone(), t.tp.clone(), t.tag_values.clone()), + ArgumentInfo::new(t.name.clone(), t.tp.clone(), t.call_id, t.tag_values.clone()), _ => return None, }) } + /// Get the function call AST ID associated with this argument. + pub fn call_id(&self) -> Option { + match self { + Self::Operation(t) => t.call_id, + Self::This(t) => t.call_id, + Self::Argument(t) => t.call_id, + Self::InsertionPoint(t) => t.call_id, + _ => None, + } + } + /// `ArgumentInfo` setter. Returns bool indicating whether the operation was possible /// or was skipped. pub fn set_argument_info(&mut self, argument_info: ArgumentInfo) -> bool { match self { + Self::Operation(t) => { + t.call_id = argument_info.call_id; + true + } Self::This(t) => { t.tp = argument_info.tp; + t.call_id = argument_info.call_id; true } Self::Argument(t) => { t.name = argument_info.name; t.tp = argument_info.tp; + t.call_id = argument_info.call_id; t.tag_values = argument_info.tag_values; true } Self::InsertionPoint(t) => { t.name = argument_info.name; t.tp = argument_info.tp; + t.call_id = argument_info.call_id; t.tag_values = argument_info.tag_values; true } @@ -165,7 +192,7 @@ impl Kind { match self { Self::Root => "Root", Self::Chained => "Chained", - Self::Operation => "Operation", + Self::Operation(_) => "Operation", Self::This(_) => "This", Self::Argument(_) => "Argument", Self::Token => "Token", @@ -184,6 +211,38 @@ impl Default for Kind { } +// ================= +// === Operation === +// ================= + +/// Kind representing an operation (operator or function) of parent Infix, Section or Prefix. +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +pub struct Operation { + /// The AST ID of function application that this operation is part of. If this is an access + /// chain operation (e.g. `method.call arg`), the call will be the outermost expression + /// containing all arguments. For other infix operators (not access), the call will be the + /// infix expression containing two arguments. + pub call_id: Option, +} + + +// === Setters === + +impl Operation { + /// Set operation `call_id` field. See [`Operation::call_id`] for more information. + pub fn with_call_id(mut self, call_id: Option) -> Self { + self.call_id = call_id; + self + } +} + +impl From for Kind { + fn from(t: Operation) -> Self { + Self::Operation(t) + } +} + + // ============ // === This === @@ -196,6 +255,7 @@ impl Default for Kind { pub struct This { pub removable: bool, pub tp: Option, + pub call_id: Option, } @@ -232,6 +292,11 @@ impl This { self.tp = tp; self } + + pub fn with_call_id(mut self, call_id: Option) -> Self { + self.call_id = call_id; + self + } } impl From for Kind { @@ -254,6 +319,7 @@ pub struct Argument { pub removable: bool, pub name: Option, pub tp: Option, + pub call_id: Option, pub tag_values: Vec, } @@ -286,6 +352,10 @@ impl Argument { self.tp = tp; self } + pub fn with_call_id(mut self, call_id: Option) -> Self { + self.call_id = call_id; + self + } } impl From for Kind { @@ -310,6 +380,7 @@ pub struct InsertionPoint { pub kind: InsertionPointType, pub name: Option, pub tp: Option, + pub call_id: Option, pub tag_values: Vec, } diff --git a/app/gui/src/controller.rs b/app/gui/src/controller.rs index bed0f2f2ebcc..c4058851e29a 100644 --- a/app/gui/src/controller.rs +++ b/app/gui/src/controller.rs @@ -20,6 +20,7 @@ pub mod visualization; pub use self::ide::Ide; pub use graph::executed::Handle as ExecutedGraph; +pub use graph::widget::Controller as Widget; pub use graph::Handle as Graph; pub use module::Handle as Module; pub use project::Project; diff --git a/app/gui/src/controller/graph.rs b/app/gui/src/controller/graph.rs index 04b4847f33f1..956b7051bedd 100644 --- a/app/gui/src/controller/graph.rs +++ b/app/gui/src/controller/graph.rs @@ -36,6 +36,7 @@ use span_tree::SpanTree; // ============== pub mod executed; +pub mod widget; pub use double_representation::graph::Id; pub use double_representation::graph::LocationHint; diff --git a/app/gui/src/controller/graph/executed.rs b/app/gui/src/controller/graph/executed.rs index 2611ab95486e..081d0559343e 100644 --- a/app/gui/src/controller/graph/executed.rs +++ b/app/gui/src/controller/graph/executed.rs @@ -297,6 +297,11 @@ impl Handle { self.graph.borrow().clone_ref() } + /// Get suggestion database from currently active graph. + pub fn suggestion_db(&self) -> Rc { + self.graph.borrow().suggestion_db.clone() + } + /// Get a full qualified name of the module in the [`graph`]. The name is obtained from the /// module's path and the `project` name. pub fn module_qualified_name(&self, project: &dyn model::project::API) -> QualifiedName { @@ -331,8 +336,8 @@ impl Handle { impl Context for Handle { fn call_info(&self, id: ast::Id, name: Option<&str>) -> Option { let lookup_registry = || { - let info = self.computed_value_info_registry().get(&id)?; - let entry = self.project.suggestion_db().lookup(info.method_call?).ok()?; + let method_call = self.computed_value_info_registry().get_method_call(&id)?; + let entry = self.project.suggestion_db().lookup(method_call).ok()?; Some(entry.invocation_info()) }; let fallback = || self.graph.borrow().call_info(id, name); diff --git a/app/gui/src/controller/graph/widget.rs b/app/gui/src/controller/graph/widget.rs new file mode 100644 index 000000000000..f8945595d1c2 --- /dev/null +++ b/app/gui/src/controller/graph/widget.rs @@ -0,0 +1,481 @@ +//! Widget controller. +//! +//! The Widget Controller is responsible for querying the language server for information about +//! the node's widget metadata or resolving it from local cache. + +use crate::prelude::*; + +use crate::controller::visualization::manager::Manager; +use crate::controller::visualization::manager::Notification; +use crate::controller::ExecutedGraph; +use crate::executor::global::spawn_stream_handler; +use crate::model::execution_context::VisualizationUpdateData; + +use engine_protocol::language_server::SuggestionId; +use ensogl::define_endpoints_2; +use ide_view::graph_editor::component::node::input::widget; +use ide_view::graph_editor::component::visualization; +use ide_view::graph_editor::component::visualization::Metadata; +use ide_view::graph_editor::data::enso::Code; +use ide_view::graph_editor::WidgetUpdate; +use ide_view::graph_editor::WidgetUpdates; + + + +/// ================= +/// === Constants === +/// ================= + +/// A module containing the widget visualization method. +const WIDGET_VISUALIZATION_MODULE: &str = "Standard.Visualization.Widgets"; +/// A name of the widget visualization method. +const WIDGET_VISUALIZATION_METHOD: &str = "get_full_annotations_json"; + + + +// =============== +// === Aliases === +// =============== + +/// An ID of a node in the graph. Always refers to the root expression. +type NodeId = ast::Id; +// An ID of any sub expression in the node, which can have a widget attached to it. +type ExpressionId = ast::Id; + + + +// ================== +// === Controller === +// ================== + +define_endpoints_2! { + Input { + /// Create or update widget query with given definition. + request_widgets(Request), + /// Remove all widget queries of given node that are not on this list. + retain_node_expressions(NodeId, HashSet), + /// Remove all widget data associated with given node. + remove_all_node_widgets(NodeId), + } + Output { + /// Emitted when the node's visualization has been set. + widget_data(NodeId, WidgetUpdates), + } +} + +/// Graph widgets controller. Handles requests for widget metadata using visualizations. Maps +/// response data to the relevant node Id updates, and dispatches them over the FRP output. +/// Guarantees that each individual query eventually receives an update. It internally caches the +/// results of the last queries, so that the metadata can be delivered to the presenter even when no +/// visualization change is necessary. +#[derive(Debug, Deref)] +pub struct Controller { + #[deref] + frp: Frp, + #[allow(dead_code)] + model: Rc>, +} + +impl Controller { + /// Constructor + pub fn new(executed_graph: ExecutedGraph) -> Self { + let (manager, manager_notifications) = Manager::new(executed_graph.clone_ref()); + let frp = Frp::new(); + + let model = Rc::new(RefCell::new(Model { + manager, + graph: executed_graph.clone_ref(), + widgets_of_node: default(), + widget_queries: default(), + })); + + let network = &frp.network; + let input = &frp.input; + let output = &frp.private.output; + + frp::extend! { network + updates_from_cache <- input.request_widgets.filter_map( + f!((definition) model.borrow_mut().request_widget(definition)) + ); + output.widget_data <+ updates_from_cache; + eval input.retain_node_expressions(((node_id, expr_ids)) { + model.borrow_mut().retain_node_expressions(*node_id, expr_ids) + }); + eval input.remove_all_node_widgets((node_id) { + model.borrow_mut().remove_all_node_widgets(*node_id) + }); + }; + + let out_widget_data = output.widget_data.clone_ref(); + let weak = Rc::downgrade(&model); + spawn_stream_handler(weak, manager_notifications, move |notification, model| { + let data = model.borrow_mut().handle_notification(notification); + if let Some(data) = data { + out_widget_data.emit(data); + } + std::future::ready(()) + }); + + Self { frp, model } + } +} + + + +// ============= +// === Model === +// ============= + +/// Model of the Widget controller. Manages the widget queries, stores responses in cache. See +/// [`Controller`] for more information. +#[derive(Debug)] +pub struct Model { + manager: Rc, + graph: ExecutedGraph, + widgets_of_node: NodeToWidgetsMapping, + /// Map of queries by the target expression ID. Required to be able to map visualization update + /// responses to the corresponding widgets. + widget_queries: HashMap, +} + +impl Model { + /// Visualization update notification handler. Updates the cache and returns the widget updates + /// when the notification provides new data. + fn handle_notification( + &mut self, + notification: Notification, + ) -> Option<(NodeId, WidgetUpdates)> { + let report_error = |message, error| { + error!("{message}: {error}"); + None + }; + + match notification { + Notification::ValueUpdate { target, data, .. } => + self.handle_visualization_value_update(target, data), + Notification::FailedToAttach { error, .. } => + report_error("Failed to attach widget visualization", error), + Notification::FailedToDetach { error, .. } => + report_error("Failed to detach widget visualization", error), + Notification::FailedToModify { error, .. } => + report_error("Failed to modify widget visualization", error), + } + } + + /// Handle visualization data update. Return widget update data. + fn handle_visualization_value_update( + &mut self, + target: ast::Id, + data: VisualizationUpdateData, + ) -> Option<(NodeId, WidgetUpdates)> { + let query_data = self.widget_queries.get_mut(&target)?; + + let (updates, errors) = VisualizationData::try_deserialize(&data); + + for error in errors { + error!("{:?}", error); + } + + trace!("Widget updates: {updates:?}"); + let updates = Rc::new(updates); + query_data.last_updates = Some(updates.clone()); + + let call_id = query_data.call_expression; + Some((query_data.node_id, WidgetUpdates { call_id, updates })) + } + + /// Handle a widget request from presenter. Returns the widget updates if the request can be + /// immediately fulfilled from the cache. + fn request_widget(&mut self, request: &Request) -> Option<(NodeId, WidgetUpdates)> { + let suggestion_db = self.graph.suggestion_db(); + let suggestion = suggestion_db.lookup(request.call_suggestion).ok()?; + + use std::collections::hash_map::Entry; + match self.widget_queries.entry(request.target_expression) { + Entry::Occupied(mut occupied) => { + let query = occupied.get_mut(); + if query.node_id != request.node_id { + self.widgets_of_node.remove_widget(query.node_id, request.target_expression); + self.widgets_of_node.insert_widget(request.node_id, request.target_expression); + } + + let visualization_modified = query.update(&suggestion, request); + if visualization_modified { + trace!("Updating widget visualization for {}", request.target_expression); + query.request_visualization(&self.manager, request.target_expression); + + // The request is now pending. Once the request completes, the widget update + // will happen in the response handler. + None + } else { + // In the event that the visualization was not modified, we want to respond with + // the last known visualization data. Each widget request needs to be responded + // to, otherwise the widget might not be displayed after the widget view has + // been temporarily removed and created again. + query.last_updates() + } + } + Entry::Vacant(vacant) => { + self.widgets_of_node.insert_widget(request.node_id, request.target_expression); + + let query = vacant.insert(QueryData::new(&suggestion, request)); + trace!("Registering widget visualization for {}", request.target_expression); + query.request_visualization(&self.manager, request.target_expression); + + // The request is now pending. Once the request completes, the widget update will + // happen in the response handler. + None + } + } + } + + /// Remove all widget queries of given node that are attached to expressions outside of provided + /// list. No widget update is emitted after a query is cleaned up. + fn retain_node_expressions(&mut self, node_id: NodeId, expressions: &HashSet) { + self.widgets_of_node.retain_node_widgets(node_id, expressions, |expr_id| { + self.manager.remove_visualization(expr_id); + }); + } + + /// Remove all widget queries of given node. No widget update is emitted after a query is + /// cleaned up. + fn remove_all_node_widgets(&mut self, node_id: NodeId) { + for expr_id in self.widgets_of_node.remove_node_widgets(node_id) { + self.manager.remove_visualization(expr_id); + } + } +} + + + +/// ============================ +/// === NodeToWidgetsMapping === +/// ============================ + +/// A map of widgets attached to nodes. Used to perform cleanup of node widget queries when node is +/// removed. +#[derive(Debug, Default)] +struct NodeToWidgetsMapping { + attached_widgets: HashMap>, +} + +impl NodeToWidgetsMapping { + fn remove_widget(&mut self, node_id: NodeId, target: ast::Id) { + self.attached_widgets.entry(node_id).and_modify(|exprs| { + let Some(index) = exprs.iter().position(|e| *e == target) else { return }; + exprs.swap_remove(index); + }); + } + + fn insert_widget(&mut self, node_id: NodeId, target: ast::Id) { + self.attached_widgets.entry(node_id).or_default().push(target); + } + + fn retain_node_widgets( + &mut self, + node_id: NodeId, + remaining_expressions: &HashSet, + mut on_remove: impl FnMut(ExpressionId), + ) { + if let Some(registered) = self.attached_widgets.get_mut(&node_id) { + registered.retain(|expr_id| { + let retained = remaining_expressions.contains(expr_id); + if !retained { + on_remove(*expr_id); + } + retained + }); + } + } + + fn remove_node_widgets(&mut self, node_id: NodeId) -> Vec { + self.attached_widgets.remove(&node_id).unwrap_or_default() + } +} + + + +/// =============== +/// === Request === +/// =============== + +/// Definition of a widget request. Defines the node subexpression that the widgets will be attached +/// to, and the method call that corresponds to that expression. +#[derive(Debug, Default, Clone, Copy)] +pub struct Request { + /// The node ID of a node that contains the widget. + pub node_id: NodeId, + /// Expression of the whole method call. Only used to correlate the visualization response with + /// the widget view. + pub call_expression: ExpressionId, + /// Target (`self`) argument in the call expression. Used as a visualization target. + pub target_expression: ExpressionId, + /// The suggestion ID of the method that this call refers to. + pub call_suggestion: SuggestionId, +} + + + +/// ================= +/// === QueryData === +/// ================= + +/// Data of ongoing widget query. Defines which expressions a visualization query is attached to, +/// and maintains enough data to correlate the response with respective widget view. +#[derive(Debug)] +struct QueryData { + node_id: NodeId, + call_expression: ExpressionId, + method_name: ImString, + arguments: Vec, + last_updates: Option>>, +} + +impl QueryData { + fn new(suggestion: &enso_suggestion_database::Entry, req: &Request) -> Self { + let node_id = req.node_id; + let arguments = suggestion.arguments.iter().map(|arg| arg.name.clone().into()).collect(); + let method_name = suggestion.name.clone().into(); + let call_expression = req.call_expression; + let last_updates = None; + QueryData { node_id, arguments, method_name, call_expression, last_updates } + } + + /// Update existing query data on new request. Returns true if the visualization query needs to + /// be updated. + fn update(&mut self, suggestion: &enso_suggestion_database::Entry, req: &Request) -> bool { + let mut visualization_modified = false; + + if self.method_name != suggestion.name { + self.method_name = suggestion.name.clone().into(); + visualization_modified = true; + } + + let mut zipped_arguments = self.arguments.iter().zip(&suggestion.arguments); + if self.arguments.len() != suggestion.arguments.len() + || !zipped_arguments.all(|(a, b)| a == &b.name) + { + self.arguments = + suggestion.arguments.iter().map(|arg| arg.name.clone().into()).collect(); + visualization_modified = true; + } + + self.node_id = req.node_id; + self.call_expression = req.call_expression; + visualization_modified + } + + fn last_updates(&self) -> Option<(NodeId, WidgetUpdates)> { + self.last_updates.as_ref().map(|updates| { + let call_id = self.call_expression; + let updates = WidgetUpdates { call_id, updates: updates.clone() }; + (self.node_id, updates) + }) + } + + fn request_visualization(&mut self, manager: &Rc, target_expression: ast::Id) { + // When visualization is requested, remove stale queried value to prevent updates while + // language server request is pending. + self.last_updates.take(); + let vis_metadata = self.visualization_metadata(); + manager.request_visualization(target_expression, vis_metadata); + } + + /// Generate visualization metadata for this query. + fn visualization_metadata(&self) -> Metadata { + let relevant_arguments = self.arguments.split_first().map_or_default(|(_self, args)| args); + let arguments: Vec = vec![ + Self::escape_visualization_argument(&self.method_name).into(), + Self::arg_sequence(relevant_arguments).into(), + ]; + + let preprocessor = visualization::instance::PreprocessorConfiguration { + module: WIDGET_VISUALIZATION_MODULE.into(), + method: WIDGET_VISUALIZATION_METHOD.into(), + arguments: Rc::new(arguments), + }; + Metadata { preprocessor } + } + + /// Escape a string to be used as a visualization argument. Transforms the string into an enso + /// expression with string literal. + fn escape_visualization_argument(arg: &str) -> String { + let segment = ast::SegmentPlain { value: arg.into() }; + let text = ast::TextLineRaw { text: vec![segment.into()] }; + text.repr() + } + + /// Escape a list of strings to be used as a visualization argument. Transforms the strings into + /// an enso expression with a list of string literals. + fn arg_sequence(args: &[ImString]) -> String { + let mut buffer = String::from("["); + for (i, arg) in args.iter().enumerate() { + if i > 0 { + buffer.push_str(", "); + } + buffer.push_str(&Self::escape_visualization_argument(arg)); + } + buffer.push(']'); + buffer + } +} + + + +/// =============================== +/// === WidgetVisualizationData === +/// =============================== + +/// A type representing the data received from the widget visualization for a single widget. +/// +/// The structure of this struct is dictated by the expected widget visualization JSON result shape. +#[derive(Debug, serde::Deserialize)] +struct VisualizationData { + constructor: widget::Kind, + display: VisualizationDataDisplay, + values: Vec, +} + +#[derive(Debug, serde::Deserialize)] +struct VisualizationDataDisplay { + constructor: widget::Display, +} + +impl VisualizationData { + fn into_metadata(self) -> widget::Metadata { + let kind = self.constructor; + let display = self.display.constructor; + let dynamic_entries = self.values.iter().map(Into::into).collect(); + widget::Metadata { kind, display, dynamic_entries } + } + + /// Try to deserialize widget visualization update data. If deserialization fails for only part + /// of the response, the rest of the response is still processed, while errors are returned + /// separately for each failed widget. + fn try_deserialize(data: &VisualizationUpdateData) -> (Vec, Vec) { + let arguments: Vec<(String, serde_json::Value)> = match serde_json::from_slice(data) { + Ok(args) => args, + Err(err) => { + let err = err + .context("Failed to deserialize a list of arguments in widget response") + .into(); + return (default(), vec![err]); + } + }; + + let updates = arguments.into_iter().map( + |(argument_name, meta_json)| -> FallibleResult { + let deserialized = serde_json::from_value(meta_json); + let deserialized: Option = deserialized.map_err(|e| { + let message = + format!("Failed to deserialize widget data for argument '{argument_name}'"); + e.context(message) + })?; + let meta = deserialized.map(VisualizationData::into_metadata); + Ok(WidgetUpdate { argument_name, meta }) + }, + ); + + updates.partition_result() + } +} diff --git a/app/gui/src/controller/visualization.rs b/app/gui/src/controller/visualization.rs index 0cd07236ccdd..53f4f8e7910f 100644 --- a/app/gui/src/controller/visualization.rs +++ b/app/gui/src/controller/visualization.rs @@ -14,6 +14,13 @@ use ide_view::graph_editor::component::visualization::java_script::Sources; use std::rc::Rc; +// ============== +// === Export === +// ============== + +pub mod manager; + + // ============= // === Error === diff --git a/app/gui/src/presenter/graph/visualization/manager.rs b/app/gui/src/controller/visualization/manager.rs similarity index 99% rename from app/gui/src/presenter/graph/visualization/manager.rs rename to app/gui/src/controller/visualization/manager.rs index de23ba9fe15a..7441a85744bd 100644 --- a/app/gui/src/presenter/graph/visualization/manager.rs +++ b/app/gui/src/controller/visualization/manager.rs @@ -273,7 +273,7 @@ impl Manager { /// Request setting a given visualization on the node. /// /// Note that `[Manager]` allows setting at most one visualization per expression. Subsequent - /// calls will chnge previous visualization to the a new one. + /// calls will change previous visualization to the a new one. pub fn request_visualization(self: &Rc, target: ast::Id, requested: Metadata) { self.set_visualization(target, Some(requested)) } @@ -664,7 +664,7 @@ mod tests { // If visualization changes ID, then we need to use detach-attach API. // We don't attach it separately, as Manager identifies visualizations by their - // expression id rather than visualization id. + // expression ID rather than visualization ID. let desired_vis_3 = Desired { visualization_id: VisualizationId::from_u128(900), expression_id: node_id, diff --git a/app/gui/src/model/execution_context.rs b/app/gui/src/model/execution_context.rs index 4d740e92922c..1d4519c77f46 100644 --- a/app/gui/src/model/execution_context.rs +++ b/app/gui/src/model/execution_context.rs @@ -117,7 +117,12 @@ impl ComputedValueInfoRegistry { /// Look up the registry for information about given expression. pub fn get(&self, id: &ExpressionId) -> Option> { - self.map.borrow_mut().get(id).cloned() + self.map.borrow().get(id).cloned() + } + + /// Look up the registry for method call suggestion ID for given expression. + pub fn get_method_call(&self, id: &ExpressionId) -> Option { + self.map.borrow().get(id)?.method_call } /// Obtain a `Future` with data from this registry. If data is not available yet, the future @@ -279,8 +284,9 @@ impl From for MethodPointer { impl From<&QualifiedMethodPointer> for MethodPointer { fn from(qualified_method_pointer: &QualifiedMethodPointer) -> Self { - let module = qualified_method_pointer.module.clone().into(); - let defined_on_type = qualified_method_pointer.defined_on_type.clone().into(); + let module = qualified_method_pointer.module.to_string_with_main_segment(); + let defined_on_type = + qualified_method_pointer.defined_on_type.to_string_with_main_segment(); let name = qualified_method_pointer.name.name().to_owned(); MethodPointer { module, defined_on_type, name } } diff --git a/app/gui/src/model/execution_context/synchronized.rs b/app/gui/src/model/execution_context/synchronized.rs index 86dc0d5b2f05..6912d0d7c6ff 100644 --- a/app/gui/src/model/execution_context/synchronized.rs +++ b/app/gui/src/model/execution_context/synchronized.rs @@ -408,7 +408,7 @@ pub mod test { expect_call!(ls.push_to_execution_context(id,stack_item) => Ok(())); } - /// Generates a mock update for a random expression id. + /// Generates a mock update for a random expression ID. /// /// It will set the typename of the expression to mock typename. pub fn mock_expression_update() -> language_server::ExpressionUpdate { @@ -419,7 +419,7 @@ pub mod test { /// Generates a mock update for a single expression. /// - /// The updated expression id will be random. The typename will be mock typename. + /// The updated expression ID will be random. The typename will be mock typename. pub fn mock_expression_updates(data: &MockData) -> ExpressionUpdates { ExpressionUpdates { context_id: data.context_id, diff --git a/app/gui/src/presenter/graph.rs b/app/gui/src/presenter/graph.rs index 88f08f586bad..2da93502406f 100644 --- a/app/gui/src/presenter/graph.rs +++ b/app/gui/src/presenter/graph.rs @@ -4,16 +4,19 @@ use crate::prelude::*; use enso_web::traits::*; +use crate::controller::graph::widget::Request as WidgetRequest; use crate::controller::upload::NodeFromDroppedFileHandler; use crate::executor::global::spawn_stream_handler; use crate::presenter::graph::state::State; +use engine_protocol::language_server::SuggestionId; use enso_frp as frp; use futures::future::LocalBoxFuture; use ide_view as view; use ide_view::graph_editor::component::node as node_view; use ide_view::graph_editor::component::visualization as visualization_view; use ide_view::graph_editor::EdgeEndpoint; +use view::graph_editor::WidgetUpdates; // ============== @@ -81,6 +84,7 @@ struct Model { view: view::graph_editor::GraphEditor, state: Rc, _visualization: Visualization, + widget: controller::Widget, _execution_stack: CallStack, } @@ -97,6 +101,7 @@ impl Model { view.clone_ref(), state.clone_ref(), ); + let widget = controller::Widget::new(controller.clone_ref()); let execution_stack = CallStack::new(controller.clone_ref(), view.clone_ref(), state.clone_ref()); Self { @@ -105,6 +110,7 @@ impl Model { view, state, _visualization: visualization, + widget, _execution_stack: execution_stack, } } @@ -185,11 +191,43 @@ impl Model { ); } + /// Update the widget target expression of a node. When this widget can be requested right now, + /// return the request structure. + fn update_widget_request_data( + &self, + call_expression: ast::Id, + target_expression: ast::Id, + ) -> Option { + let node_id = self + .state + .update_from_view() + .set_call_expression_target_id(call_expression, Some(target_expression))?; + let method_id = self.expression_method_suggestion(call_expression)?; + + Some(WidgetRequest { + node_id, + call_expression, + target_expression, + call_suggestion: method_id, + }) + } + + /// Map widget controller update data to the node views. + fn map_widget_update_data( + &self, + node_id: AstNodeId, + updates: WidgetUpdates, + ) -> Option<(ViewNodeId, WidgetUpdates)> { + let node_id = self.state.view_id_of_ast_node(node_id)?; + Some((node_id, updates)) + } + /// Node was removed in view. fn node_removed(&self, id: ViewNodeId) { self.log_action( || { let ast_id = self.state.update_from_view().remove_node(id)?; + self.widget.remove_all_node_widgets(ast_id); Some(self.controller.graph().remove_node(ast_id)) }, "remove node", @@ -252,25 +290,15 @@ impl Model { ) -> Vec<(ViewNodeId, ast::Id, Option)> { let subexpressions = self.state.expressions_of_node(node); subexpressions - .iter() + .into_iter() .map(|id| { - let a_type = self.expression_type(*id); - self.state.update_from_controller().set_expression_type(*id, a_type.clone()); - (node, *id, a_type) + let a_type = self.expression_type(id); + self.state.update_from_controller().set_expression_type(id, a_type.clone()); + (node, id, a_type) }) .collect() } - /// Extract all method pointers for subexpressions, update the state, and return events updating - /// view for expressions where method pointer actually changed. - fn all_method_pointers_of_node( - &self, - node: ViewNodeId, - ) -> Vec<(ast::Id, Option)> { - let subexpressions = self.state.expressions_of_node(node); - subexpressions.iter().filter_map(|id| self.refresh_expression_method_pointer(*id)).collect() - } - /// Refresh type of the given expression. /// /// If the view update is required, the GraphEditor's FRP input event is returned. @@ -284,18 +312,34 @@ impl Model { Some((node_view, id, a_type)) } - /// Refresh method pointer of the given expression. + /// Refresh method pointer of the given expression in order to check if its widgets require an + /// update and should be queried. /// - /// If the view update is required, the GraphEditor's FRP input event is returned. - fn refresh_expression_method_pointer( - &self, - id: ast::Id, - ) -> Option<(ast::Id, Option)> { - let method_pointer = self.expression_method(id); - self.state + /// If the view update is required, the widget query data is returned. + fn refresh_expression_widgets(&self, expr_id: ast::Id) -> Option<(ast::Id, ast::Id)> { + let suggestion_id = self.expression_method_suggestion(expr_id)?; + let method_pointer = self.suggestion_method_pointer(suggestion_id); + let (_, method_target_id) = self + .state .update_from_controller() - .set_expression_method_pointer(id, method_pointer.clone())?; - Some((id, method_pointer)) + .set_expression_method_pointer(expr_id, method_pointer)?; + Some((expr_id, method_target_id?)) + } + + /// Extract all widget queries for all node subexpressions that require an update. Returns all + fn refresh_all_widgets_of_node( + &self, + node: ViewNodeId, + ) -> (Option<(AstNodeId, HashSet)>, Vec<(ast::Id, ast::Id)>) { + let subexpressions = self.state.expressions_of_node(node); + let expressions_map = subexpressions.iter().cloned().collect(); + let node_id = self.state.ast_node_id_of_view(node); + let node_expressions = node_id.map(|id| (id, expressions_map)); + let queries = subexpressions + .into_iter() + .filter_map(|id| self.refresh_expression_widgets(id)) + .collect(); + (node_expressions, queries) } fn refresh_node_error( @@ -314,11 +358,18 @@ impl Model { Some(view::graph_editor::Type(info.typename.as_ref()?.clone_ref())) } - /// Extract the expression's current method pointer from controllers. - fn expression_method(&self, id: ast::Id) -> Option { + /// Extract the expression's current suggestion entry from controllers. + fn expression_method_suggestion(&self, id: ast::Id) -> Option { let registry = self.controller.computed_value_info_registry(); - let method_id = registry.get(&id)?.method_call?; - let suggestion_db = self.controller.graph().suggestion_db.clone_ref(); + registry.get(&id)?.method_call + } + + /// Extract the expression's current method pointer from controllers. + fn suggestion_method_pointer( + &self, + method_id: SuggestionId, + ) -> Option { + let suggestion_db = self.controller.suggestion_db(); let method = suggestion_db.lookup_method_ptr(method_id).ok()?; Some(view::graph_editor::MethodPointer(Rc::new(method))) } @@ -555,7 +606,8 @@ impl Graph { fn init(self, project_view: &view::project::View) -> Self { let network = &self.network; let model = &self.model; - let view = &self.model.view.frp; + let view = &model.view.frp; + frp::extend! { network update_view <- source::<()>(); // Position initialization should go before emitting `update_data` event. @@ -613,16 +665,14 @@ impl Graph { reset_node_types <- any(update_node_expression, init_node_expression)._0(); set_expression_type <= reset_node_types.map(f!((view_id) model.all_types_of_node(*view_id))); - set_method_pointer <= reset_node_types.map(f!((view_id) model.all_method_pointers_of_node(*view_id))); view.set_expression_usage_type <+ set_expression_type; - view.set_method_pointer <+ set_method_pointer; update_expressions <- source::>(); update_expression <= update_expressions; view.set_expression_usage_type <+ update_expression.filter_map(f!((id) model.refresh_expression_type(*id))); - view.set_method_pointer <+ update_expression.filter_map(f!((id) model.refresh_expression_method_pointer(*id))); view.set_node_error_status <+ update_expression.filter_map(f!((id) model.refresh_node_error(*id))); + self.init_widgets(reset_node_types, update_expression.clone_ref()); // === Changes from the View === @@ -651,6 +701,46 @@ impl Graph { self } + /// Initialize the FRP network for querying and updating expression widgets. + fn init_widgets( + &self, + reset_node_types: frp::Stream, + update_expression: frp::Stream, + ) { + let network = &self.network; + let model = &self.model; + let view = &model.view.frp; + let widget = &model.widget; + + frp::extend! { network + widget_refresh <- reset_node_types.map( + f!((view_id) model.refresh_all_widgets_of_node(*view_id)) + ); + } + frp::extend! { network + widgets_to_update <- any(...); + widgets_to_update <+ widget_refresh._1().iter(); + } + frp::extend! { network + widgets_to_update <+ update_expression.filter_map( + f!((id) model.refresh_expression_widgets(*id)) + ); + widgets_to_update <+ view.widgets_requested.map(|(_, call, target)| (*call, *target)); + } + frp::extend! { network + widget_request <- widgets_to_update.filter_map( + f!(((call, target)) model.update_widget_request_data(*call, *target)) + ); + widget.request_widgets <+ widget_request; + } + frp::extend! { network + widget.retain_node_expressions <+ widget_refresh._0().unwrap(); + view.update_node_widgets <+ widget.widget_data.filter_map( + f!(((id, updates)) model.map_widget_update_data(*id, updates.clone())) + ); + } + } + fn setup_controller_notification_handlers( &self, update_view: frp::Source<()>, @@ -686,12 +776,12 @@ impl Graph { self.model.state.view_id_of_ast_node(id) } - /// Get the ast id of given node view. + /// Get the AST ID of given node view. pub fn ast_node_of_view(&self, id: ViewNodeId) -> Option { self.model.state.ast_node_id_of_view(id) } - /// Assign a node view to the given AST id. Since next update, the presenter will share the + /// Assign a node view to the given AST ID. Since next update, the presenter will share the /// node content between the controllers and the view. pub fn assign_node_view_explicitly(&self, view_id: ViewNodeId, ast_id: AstNodeId) { self.model.state.assign_node_view_explicitly(view_id, ast_id); diff --git a/app/gui/src/presenter/graph/state.rs b/app/gui/src/presenter/graph/state.rs index 41d47de7096c..58aa4e4551ff 100644 --- a/app/gui/src/presenter/graph/state.rs +++ b/app/gui/src/presenter/graph/state.rs @@ -55,7 +55,7 @@ impl Default for Node { /// The set of node states. /// -/// This structure allows to access data of any node by Ast id, or view id. It also keeps list +/// This structure allows to access data of any node by Ast ID, or view id. It also keeps list /// of the AST nodes with no view assigned, and allows to assign View Id to the next one. #[derive(Clone, Debug, Default)] pub struct Nodes { @@ -69,12 +69,12 @@ pub struct Nodes { } impl Nodes { - /// Get the state of the node by Ast id. + /// Get the state of the node by Ast ID. pub fn get(&self, id: AstNodeId) -> Option<&Node> { self.nodes.get(&id) } - /// Get mutable reference of the node's state by Ast id. + /// Get mutable reference of the node's state by Ast ID. pub fn get_mut(&mut self, id: AstNodeId) -> Option<&mut Node> { self.nodes.get_mut(&id) } @@ -103,7 +103,7 @@ impl Nodes { }) } - /// Get the AST id of the node represented by given view. Returns None, if the node view does + /// Get the AST ID of the node represented by given view. Returns None, if the node view does /// not represent any AST node. pub fn ast_id_of_view(&self, view_id: ViewNodeId) -> Option { self.ast_node_by_view_id.get(&view_id).copied() @@ -152,7 +152,7 @@ impl Nodes { removed_views } - /// Remove node represented by given view (if any) and return it's AST id. + /// Remove node represented by given view (if any) and return it's AST ID. pub fn remove_node(&mut self, node: ViewNodeId) -> Option { let ast_id = self.ast_node_by_view_id.remove(&node)?; self.nodes.remove(&ast_id); @@ -241,6 +241,8 @@ pub struct Expression { pub expression_type: Option, /// A pointer to the method called by this expression. pub method_pointer: Option, + /// A AST ID of `self` argument associated with a method call represented by this expression. + pub target_id: Option, } /// The data of node's expressions. @@ -312,12 +314,12 @@ pub struct State { } impl State { - /// Get node's view id by the AST id. + /// Get node's view id by the AST ID. pub fn view_id_of_ast_node(&self, node: AstNodeId) -> Option { self.nodes.borrow().get(node).and_then(|n| n.view_id) } - /// Get node's AST id by the view id. + /// Get node's AST ID by the view id. pub fn ast_node_id_of_view(&self, node: ViewNodeId) -> Option { self.nodes.borrow().ast_id_of_view(node) } @@ -624,20 +626,18 @@ impl<'a> ControllerChange<'a> { } /// Set the new expression's method pointer. If the method pointer actually changes, the - /// to-be-updated view is returned. + /// to-be-updated view and target (`self` argument) AST ID is returned. pub fn set_expression_method_pointer( &self, id: ast::Id, method_ptr: Option, - ) -> Option { + ) -> Option<(ViewNodeId, Option)> { let mut expressions = self.expressions.borrow_mut(); - let to_update = expressions.get_mut(id).filter(|d| d.method_pointer != method_ptr); - if let Some(displayed) = to_update { - displayed.method_pointer = method_ptr; - self.nodes.borrow().get(displayed.node).and_then(|node| node.view_id) - } else { - None - } + let displayed = expressions.get_mut(id).filter(|d| d.method_pointer != method_ptr)?; + displayed.method_pointer = method_ptr; + let nodes = self.nodes.borrow(); + let node = nodes.get(displayed.node)?; + Some((node.view_id?, displayed.target_id)) } } @@ -667,7 +667,7 @@ pub struct ViewChange<'a> { impl<'a> ViewChange<'a> { /// Set the new node position. If the node position actually changed, the AST node to-be-updated - /// id is returned. + /// ID is returned. pub fn set_node_position(&self, id: ViewNodeId, new_position: Vector2) -> Option { let mut nodes = self.nodes.borrow_mut(); let ast_id = nodes.ast_id_of_view(id)?; @@ -680,12 +680,12 @@ impl<'a> ViewChange<'a> { } } - /// Remove the node, and returns its AST id. + /// Remove the node, and returns its AST ID. pub fn remove_node(&self, id: ViewNodeId) -> Option { self.nodes.borrow_mut().remove_node(id) } - /// Set the new node visualization. If the visualization actually changes, the AST id of the + /// Set the new node visualization. If the visualization actually changes, the AST ID of the /// affected node is returned. pub fn set_node_visualization( &self, @@ -703,7 +703,7 @@ impl<'a> ViewChange<'a> { } } - /// Mark the node as skipped and return its AST id. Returns `None` if no changes to the + /// Mark the node as skipped and return its AST ID. Returns `None` if no changes to the /// expression are needed. pub fn set_node_skip(&self, id: ViewNodeId, skip: bool) -> Option { let mut nodes = self.nodes.borrow_mut(); @@ -717,7 +717,7 @@ impl<'a> ViewChange<'a> { } } - /// Mark the node as frozen and return its AST id. Returns `None` if no changes to the + /// Mark the node as frozen and return its AST ID. Returns `None` if no changes to the /// expression are needed. pub fn set_node_freeze(&self, id: ViewNodeId, freeze: bool) -> Option { let mut nodes = self.nodes.borrow_mut(); @@ -748,7 +748,20 @@ impl<'a> ViewChange<'a> { } } - /// Determine if an expression span change is valid and has any effect. Returns node AST id. + /// Set AST ID of target argument (`self`) associated with a given call expression. Returns + /// affected node id when the expression was found, even when the target id is not modified. + pub fn set_call_expression_target_id( + &self, + expression: ast::Id, + target_id: Option, + ) -> Option { + let mut expressions = self.expressions.borrow_mut(); + let displayed = expressions.get_mut(expression)?; + displayed.target_id = target_id; + Some(displayed.node) + } + + /// Determine if an expression span change is valid and has any effect. Returns node AST ID. /// Returns `None` if no changes to the expression are needed or when the span doesn't exist. pub fn check_node_expression_span_update( &self, @@ -1033,8 +1046,11 @@ mod tests { name: "foo".to_string(), }; let method_ptr = Some(view::graph_editor::MethodPointer::from(method_ptr)); - assert_eq!(updater.set_expression_method_pointer(expr, method_ptr.clone()), Some(view)); + assert_eq!( + updater.set_expression_method_pointer(expr, method_ptr.clone()), + Some((view, None)) + ); assert_eq!(updater.set_expression_method_pointer(expr, method_ptr), None); - assert_eq!(updater.set_expression_method_pointer(expr, None), Some(view)); + assert_eq!(updater.set_expression_method_pointer(expr, None), Some((view, None))); } } diff --git a/app/gui/src/presenter/graph/visualization.rs b/app/gui/src/presenter/graph/visualization.rs index c8983000067e..9bf1036bbf03 100644 --- a/app/gui/src/presenter/graph/visualization.rs +++ b/app/gui/src/presenter/graph/visualization.rs @@ -3,10 +3,11 @@ use crate::prelude::*; +use crate::controller::visualization::manager; +use crate::controller::visualization::manager::Manager; use crate::executor::global::spawn_stream_handler; use crate::model::execution_context::VisualizationUpdateData; use crate::presenter::graph; -use crate::presenter::graph::visualization::manager::Manager; use crate::presenter::graph::AstNodeId; use crate::presenter::graph::ViewNodeId; @@ -16,13 +17,6 @@ use ide_view::graph_editor::component::node as node_view; use ide_view::graph_editor::component::visualization as visualization_view; -// ============== -// === Export === -// ============== - -pub mod manager; - - // ============= // === Model === @@ -187,8 +181,8 @@ impl Visualization { frp::extend! { network eval view.visualization_shown (((node, metadata)) model.visualization_shown(*node, metadata.clone())); - eval view.visualization_hidden ((node) model.node_removed(*node)); - eval view.node_removed ((node) model.visualization_hidden(*node)); + eval view.visualization_hidden ((node) model.visualization_hidden(*node)); + eval view.node_removed ((node) model.node_removed(*node)); eval view.visualization_preprocessor_changed (((node, preprocessor)) model.visualization_preprocessor_changed(*node, preprocessor.clone_ref())); eval view.set_node_error_status (((node, error)) model.error_on_node_changed(*node, error)); diff --git a/app/gui/src/presenter/searcher.rs b/app/gui/src/presenter/searcher.rs index 6d5811332144..76ab0194b3b3 100644 --- a/app/gui/src/presenter/searcher.rs +++ b/app/gui/src/presenter/searcher.rs @@ -538,7 +538,7 @@ impl Searcher { /// This method takes `self`, as the presenter (with the searcher view) should be dropped once /// editing finishes. The `entry_id` might be none in case where the searcher should accept /// the node input without any entry selected. If the commitment results in creating a new - /// node, its AST id is returned. + /// node, its AST ID is returned. #[profile(Task)] pub fn commit_editing(self, entry_id: Option) -> Option { self.model.commit_editing(entry_id) @@ -549,7 +549,7 @@ impl Searcher { /// This method takes `self`, as the presenter (with the searcher view) should be dropped once /// editing finishes. The `entry_id` might be none in case where the user want to accept /// the node input without any entry selected. If the commitment results in creating a new - /// node, its AST id is returned. + /// node, its AST ID is returned. pub fn expression_accepted( self, entry_id: Option, diff --git a/app/gui/src/tests.rs b/app/gui/src/tests.rs index bc0ee3629b73..f1cd1196cdaa 100644 --- a/app/gui/src/tests.rs +++ b/app/gui/src/tests.rs @@ -82,13 +82,18 @@ fn span_tree_args() { let get_node = || graph.node(id).unwrap(); let get_inputs = || NodeTrees::new(&get_node().info, executed_graph).unwrap().inputs; - let get_param = - |n| get_inputs().root_ref().leaf_iter().nth(n).and_then(|node| node.argument_info()); + let get_param = |n| { + let inputs = get_inputs(); + let mut args = inputs.root_ref().leaf_iter().filter(|n| n.is_function_parameter()); + args.nth(n).and_then(|node| node.argument_info()) + }; + let expected_this_param = - model::suggestion_database::entry::to_span_tree_param(&entry.arguments[0]); + model::suggestion_database::entry::to_span_tree_param(&entry.arguments[0]) + .with_call_id(Some(id)); let expected_arg1_param = - model::suggestion_database::entry::to_span_tree_param(&entry.arguments[1]); - + model::suggestion_database::entry::to_span_tree_param(&entry.arguments[1]) + .with_call_id(Some(id)); // === Method notation, without prefix application === assert_eq!(get_node().info.expression().repr(), "Base.foo"); @@ -101,7 +106,7 @@ fn span_tree_args() { node::Kind::insertion_point().with_kind(InsertionPointType::ExpectedArgument(0)); assert!(children.is_empty()); // assert_eq!(kind,&node::Kind::from(expected_kind)); - assert_eq!(kind.argument_info(), Some(expected_arg1_param.clone())); + assert_eq!(kind.argument_info().as_ref(), Some(&expected_arg1_param)); } _ => panic!("Expected only two children in the span tree's root"), }; @@ -115,8 +120,7 @@ fn span_tree_args() { [_, second] => { let Node { children, kind, .. } = &second.node; assert!(children.is_empty()); - // assert_eq!(kind,&node::Kind::from(node::Kind::argument())); - assert_eq!(kind.argument_info(), Some(expected_arg1_param.clone())); + assert_eq!(kind.argument_info().as_ref(), Some(&expected_arg1_param)); } inputs => panic!("Expected two children in the span tree's root but got {:?}", inputs.len()), @@ -126,37 +130,39 @@ fn span_tree_args() { // === Function notation, without prefix application === assert_eq!(entry.name, "foo"); graph.set_expression(id, "foo").unwrap(); - assert_eq!(get_param(1).as_ref(), Some(&expected_this_param)); - assert_eq!(get_param(2).as_ref(), Some(&expected_arg1_param)); - assert_eq!(get_param(3).as_ref(), None); + assert_eq!(get_param(0).as_ref(), Some(&expected_this_param)); + assert_eq!(get_param(1).as_ref(), Some(&expected_arg1_param)); + assert_eq!(get_param(2).as_ref(), None); // === Function notation, with prefix application === graph.set_expression(id, "foo Base").unwrap(); - assert_eq!(get_param(1).as_ref(), Some(&expected_this_param)); - assert_eq!(get_param(2).as_ref(), Some(&expected_arg1_param)); - assert_eq!(get_param(3).as_ref(), None); + assert_eq!(get_param(0).as_ref(), Some(&expected_this_param)); + assert_eq!(get_param(1).as_ref(), Some(&expected_arg1_param)); + assert_eq!(get_param(2).as_ref(), None); // === Changed function name, should not have known parameters === graph.set_expression(id, "bar").unwrap(); + assert_eq!(get_param(0), None); assert_eq!(get_param(1), None); assert_eq!(get_param(2), None); - assert_eq!(get_param(3), None); graph.set_expression(id, "bar Base").unwrap(); - assert_eq!(get_param(1), Some(default())); - assert_eq!(get_param(2), Some(span_tree::ArgumentInfo::this(None))); - assert_eq!(get_param(3), Some(default())); // FIXME: is this correct? + assert_eq!(get_param(0), Some(span_tree::ArgumentInfo::this(None, None))); + assert_eq!(get_param(1), None); + assert_eq!(get_param(2), None); graph.set_expression(id, "Base.bar").unwrap(); - assert_eq!(get_param(1), Some(span_tree::ArgumentInfo::this(None))); - assert_eq!(get_param(2), Some(default())); - assert_eq!(get_param(3), None); + assert_eq!(get_param(0), Some(span_tree::ArgumentInfo::this(None, None))); + assert_eq!(get_param(1), Some(default())); + assert_eq!(get_param(2), None); // === Oversaturated call === graph.set_expression(id, "foo Base 10 20 30").unwrap(); - assert_eq!(get_param(1).as_ref(), Some(&expected_this_param)); - assert_eq!(get_param(2).as_ref(), Some(&expected_arg1_param)); + assert_eq!(get_param(0).as_ref(), Some(&expected_this_param)); + assert_eq!(get_param(1).as_ref(), Some(&expected_arg1_param)); + assert_eq!(get_param(2).as_ref(), Some(&default())); assert_eq!(get_param(3).as_ref(), Some(&default())); + assert_eq!(get_param(4).as_ref(), None); } diff --git a/app/gui/suggestion-database/src/entry.rs b/app/gui/suggestion-database/src/entry.rs index 5256feafc8c5..21a6f98634de 100644 --- a/app/gui/suggestion-database/src/entry.rs +++ b/app/gui/suggestion-database/src/entry.rs @@ -867,6 +867,7 @@ pub fn to_span_tree_param(param_info: &Argument) -> span_tree::ArgumentInfo { // TODO [mwu] Check if database actually do must always have both of these filled. name: Some(param_info.name.clone()), tp: Some(param_info.repr_type.clone()), + call_id: None, tag_values: param_info.tag_values.clone(), } } diff --git a/app/gui/suggestion-database/src/lib.rs b/app/gui/suggestion-database/src/lib.rs index 35258e88e5f8..bb08d7f260a8 100644 --- a/app/gui/suggestion-database/src/lib.rs +++ b/app/gui/suggestion-database/src/lib.rs @@ -417,7 +417,7 @@ impl SuggestionDatabase { /// Search the database for an entry of method identified by given id. pub fn lookup_method(&self, id: MethodId) -> Option> { - self.entries.borrow().values().cloned().find(|entry| entry.method_id().contains(&id)) + self.entries.borrow().values().find(|entry| entry.method_id().contains(&id)).cloned() } /// Search the database for an entry at `fully_qualified_name`. The parameter is expected to be diff --git a/app/gui/tests/language_server.rs b/app/gui/tests/language_server.rs index 67c5d9ce5ec4..9321b72ddb56 100644 --- a/app/gui/tests/language_server.rs +++ b/app/gui/tests/language_server.rs @@ -127,7 +127,7 @@ async fn ls_text_protocol_test() { // Setting visualization. let visualisation_id = uuid::Uuid::new_v4(); let expression_id = uuid::Uuid::parse_str("c553533e-a2b9-4305-9f12-b8fe7781f933"); - let expression_id = expression_id.expect("Couldn't parse expression id."); + let expression_id = expression_id.expect("Couldn't parse expression ID."); let visualization_function = "foo".to_string(); let visualization_module = "Test.Visualisation"; let expression = MethodPointer { diff --git a/app/gui/view/debug_scene/interface/src/lib.rs b/app/gui/view/debug_scene/interface/src/lib.rs index 0d6719e2f528..ace730fb663a 100644 --- a/app/gui/view/debug_scene/interface/src/lib.rs +++ b/app/gui/view/debug_scene/interface/src/lib.rs @@ -365,7 +365,7 @@ pub fn expression_mock2() -> Expression { let output_span_tree = span_tree::SpanTree::default(); let input_span_tree = span_tree::builder::TreeBuilder::new(36) .add_child(0, 14, span_tree::node::Kind::Chained, PrefixCrumb::Func) - .add_child(0, 9, span_tree::node::Kind::Operation, PrefixCrumb::Func) + .add_child(0, 9, span_tree::node::Kind::operation(), PrefixCrumb::Func) .set_ast_id(Uuid::new_v4()) .done() .add_empty_child(10, span_tree::node::InsertionPointType::BeforeTarget) @@ -379,7 +379,7 @@ pub fn expression_mock2() -> Expression { .set_ast_id(Uuid::new_v4()) .add_child(1, 19, span_tree::node::Kind::argument(), parens_cr) .set_ast_id(Uuid::new_v4()) - .add_child(0, 12, span_tree::node::Kind::Operation, PrefixCrumb::Func) + .add_child(0, 12, span_tree::node::Kind::operation(), PrefixCrumb::Func) .set_ast_id(Uuid::new_v4()) .done() .add_empty_child(13, span_tree::node::InsertionPointType::BeforeTarget) @@ -447,13 +447,14 @@ pub fn expression_mock_trim() -> Expression { ..default() }; let param0 = span_tree::ArgumentInfo { - name: Some("where".to_owned()), - tp: Some("Location".to_owned()), + name: Some("where".to_owned()), + tp: Some("Location".to_owned()), tag_values: vec![ "Location.Start".to_owned(), "Location.End".to_owned(), "Location.Both".to_owned(), ], + ..default() }; let param1 = span_tree::ArgumentInfo { name: Some("what".to_owned()), diff --git a/app/gui/view/graph-editor/src/component/breadcrumbs/breadcrumb.rs b/app/gui/view/graph-editor/src/component/breadcrumbs/breadcrumb.rs index 4762637ab2df..a278af8f9f02 100644 --- a/app/gui/view/graph-editor/src/component/breadcrumbs/breadcrumb.rs +++ b/app/gui/view/graph-editor/src/component/breadcrumbs/breadcrumb.rs @@ -242,7 +242,7 @@ impl Frp { // === BreadcrumbInfo === // ====================== -/// Breadcrumb information such as name and expression id. +/// Breadcrumb information such as name and expression ID. #[derive(Debug)] #[allow(missing_docs)] pub struct BreadcrumbInfo { @@ -266,7 +266,7 @@ pub struct BreadcrumbModel { label: text::Text, animations: Animations, style: StyleWatch, - /// Breadcrumb information such as name and expression id. + /// Breadcrumb information such as name and expression ID. pub info: Rc, relative_position: Rc>>, outputs: FrpOutputs, diff --git a/app/gui/view/graph-editor/src/component/node.rs b/app/gui/view/graph-editor/src/component/node.rs index f6366907630d..77aa9b64f1aa 100644 --- a/app/gui/view/graph-editor/src/component/node.rs +++ b/app/gui/view/graph-editor/src/component/node.rs @@ -12,6 +12,7 @@ use crate::selection::BoundingBox; use crate::tooltip; use crate::view; use crate::Type; +use crate::WidgetUpdates; use super::edge; use enso_frp as frp; @@ -306,6 +307,7 @@ ensogl::define_endpoints_2! { /// `set_expression` instead. In case the usage type is set to None, ports still may be /// colored if the definition type was present. set_expression_usage_type (Crumbs,Option), + update_widgets (WidgetUpdates), set_output_expression_visibility (bool), set_vcs_status (Option), /// Show visualization preview until either editing of the node is finished or the @@ -353,6 +355,10 @@ ensogl::define_endpoints_2! { /// [`visualization_visible`] is updated. Please remember, that the [`position`] is not /// immediately updated, only during the Display Object hierarchy update bounding_box (BoundingBox), + /// A set of widgets attached to a method requires metadata to be queried. The tuple + /// contains the ID of the call expression the widget is attached to, and the ID of that + /// call's target expression (`self` or first argument). + requested_widgets (ast::Id, ast::Id), } } @@ -745,8 +751,11 @@ impl Node { eval input.set_expression ((a) model.set_expression(a)); out.expression <+ model.input.frp.expression; out.expression_span <+ model.input.frp.on_port_code_update; + out.requested_widgets <+ model.input.frp.requested_widgets; + model.input.set_connected <+ input.set_input_connected; model.input.set_disabled <+ input.set_disabled; + model.input.update_widgets <+ input.update_widgets; model.output.set_expression_visibility <+ input.set_output_expression_visibility; diff --git a/app/gui/view/graph-editor/src/component/node/input/area.rs b/app/gui/view/graph-editor/src/component/node/input/area.rs index f586f6ca2d43..1eecc62d851a 100644 --- a/app/gui/view/graph-editor/src/component/node/input/area.rs +++ b/app/gui/view/graph-editor/src/component/node/input/area.rs @@ -9,9 +9,11 @@ use ensogl::display::traits::*; use crate::component::type_coloring; use crate::node; use crate::node::input::port; +use crate::node::input::widget; use crate::node::profiling; use crate::view; use crate::Type; +use crate::WidgetUpdates; use enso_frp as frp; use enso_frp; @@ -102,6 +104,34 @@ impl Debug for Expression { } } +// === Pretty printing debug adapter === + +/// Debug adapter used for pretty-printing the `Expression` span tree. Can be used to print the +/// expression with detailed span-tree information. This printer is normally too verbose to be +/// a default `Debug` implementation of `Expression`, so it is hidden behind a separate adapter +/// and can be chosen by calling `expression.tree_pretty_printer()`. +pub struct ExpressionTreePrettyPrint<'a> { + expression: &'a Expression, +} + +impl<'a> Debug for ExpressionTreePrettyPrint<'a> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> fmt::Result { + let printed = self.expression.span_tree.debug_print(&self.expression.code); + f.write_str(&printed) + } +} + +impl Expression { + /// Wrap the expression into a pretty-printing adapter that implements `Debug` and prints + /// detailed span-tree information. See [`SpanTree::debug_print`] method for more details. + /// + /// Note that this printer emits multi-line output. In order for those lines to be properly + /// aligned, it should be always printed on a new line. + pub fn tree_pretty_printer(&self) -> ExpressionTreePrettyPrint<'_> { + ExpressionTreePrettyPrint { expression: self } + } +} + // === Conversions === @@ -178,10 +208,17 @@ pub struct Model { label: text::Text, expression: RefCell, id_crumbs_map: RefCell>, + widgets_map: RefCell>, styles: StyleWatch, styles_frp: StyleWatchFrp, } +#[derive(Debug, Clone, Hash, PartialEq, Eq)] +struct WidgetBind { + call_id: ast::Id, + argument_name: String, +} + impl Model { /// Constructor. #[profile(Debug)] @@ -195,6 +232,7 @@ impl Model { let expression = default(); let styles = StyleWatch::new(&app.display.default_scene.style_sheet); let styles_frp = StyleWatchFrp::new(&app.display.default_scene.style_sheet); + let widgets_map = default(); display_object.add_child(&label); display_object.add_child(&ports); ports.add_child(&header); @@ -206,6 +244,7 @@ impl Model { label, expression, id_crumbs_map, + widgets_map, styles, styles_frp, } @@ -271,6 +310,46 @@ impl Model { } } + /// Apply widget updates to widgets in this input area. + fn apply_widget_updates(&self, updates: &WidgetUpdates) { + let expression = self.expression.borrow(); + let widgets_map = self.widgets_map.borrow(); + let WidgetUpdates { call_id, updates } = updates; + for update in updates.iter() { + let argument_name = update.argument_name.to_string(); + let widget_id = WidgetBind { call_id: *call_id, argument_name }; + let crumbs = widgets_map.get(&widget_id); + + let root = expression.span_tree.root_ref(); + let port = crumbs.and_then(|crumbs| root.get_descendant(crumbs).ok()); + let widget = port.and_then(|port| port.payload.widget.clone_ref()); + + // When a widget is found, update it. Failing to find a widget is not an error, as it + // might be a widget that was removed from the expression while the request was pending. + // If it comes back, the widget data will be requested again. + if let Some(widget) = widget { + widget.set_metadata(update.meta.clone()); + } + } + } + + /// Set the visibility of all widgets in this input area. This is only a visual change, and does + /// not affect the widget's state. Widget updates are still processed when the widget is hidden. + fn set_widgets_visibility(&self, visible: bool) { + let expression = self.expression.borrow(); + let widgets_map = self.widgets_map.borrow(); + for (id, crumbs) in widgets_map.iter() { + let root = expression.span_tree.root_ref(); + let port = root.get_descendant(crumbs).ok(); + let widget = port.and_then(|port| port.payload.widget.clone_ref()); + if let Some(widget) = widget { + widget.set_visible(visible); + } else { + error!("Widget {id:?} not found for crumbs {crumbs:?}."); + } + } + } + #[profile(Debug)] fn set_label_on_new_expression(&self, expression: &Expression) { self.label.set_content(expression.viz_code.clone()); @@ -281,14 +360,17 @@ impl Model { &self, expression: &mut Expression, area_frp: &FrpEndpoints, + call_info: &CallInfoMap, ) { let mut is_header = true; + let mut id_crumbs_map = HashMap::new(); + let mut widgets_map = HashMap::new(); let builder = PortLayerBuilder::empty(&self.ports); let code = &expression.viz_code; + expression.span_tree.root_ref_mut().dfs_with_layer_data(builder, |mut node, builder| { let is_parensed = node.is_parensed(); - let last_argument = node.children.iter().rposition(|child| child.is_argument()); let skip_opr = if SKIP_OPERATIONS { node.is_operation() && !is_header } else { @@ -331,6 +413,7 @@ impl Model { let new_parent = if not_a_port { builder.parent.clone_ref() } else { + let crumbs = node.crumbs.clone_ref(); let port = &mut node; let index = local_char_offset + builder.shift; @@ -341,11 +424,11 @@ impl Model { let height = 18.0; let padded_size = Vector2(width_padded, height); let size = Vector2(width, height); - let port_shape = port.payload_mut().init_shape(size, node::HEIGHT); - let argument_info = port.argument_info(); - let port_widget = port.payload_mut().init_widget(&self.app, argument_info, node::HEIGHT); + let position_x = unit * index as f32; + + let port_shape = port.payload.init_shape(size, node::HEIGHT); - port_shape.set_x(unit * index as f32); + port_shape.set_x(position_x); if DEBUG { port_shape.set_y(DEBUG_PORT_OFFSET); } @@ -363,7 +446,6 @@ impl Model { let styles = StyleWatch::new(style_sheet); let styles_frp = &self.styles_frp; let any_type_sel_color = styles_frp.get_color(theme::code::types::any::selection); - let crumbs = port.crumbs.clone_ref(); let port_network = &port.network; frp::extend! { port_network @@ -438,31 +520,34 @@ impl Model { area_frp.source.pointer_style <+ pointer_style; } - if let Some(port_widget) = port_widget { - port_widget.set_x(unit * index as f32); - builder.parent.add_child(&port_widget); + if let Some((widget_bind, widget)) = self.init_port_widget(port, call_info) { + widgets_map.insert(widget_bind, crumbs.clone_ref()); + widget.set_x(position_x); + builder.parent.add_child(&widget); + if port.is_argument() { let range = port.payload.range(); let code = &expression.viz_code[range]; - port_widget.set_current_value(Some(code.into())); + widget.set_current_value(Some(code.into())); + } else { + widget.set_current_value(None); } - let last_arg_crumb = builder.parent_last_argument; + + let can_be_removed = call_info.is_last_argument(port); + let empty_value = if can_be_removed { "" } else { "_" }; + + let port_network = &port.network; frp::extend! { port_network - area_frp.source.on_port_code_update <+ port_widget.value_changed.map( - f!([crumbs](v) { - let crumbs = crumbs.clone_ref(); - let is_last_argument = crumbs.last().copied() == last_arg_crumb; - (crumbs.clone_ref(), v.clone().unwrap_or_else(|| { - if is_last_argument { default() } else { "_".into() } - })) - }) - ); + code_update <- widget.value_changed.map(f!([crumbs](value) { + let expression = value.clone().unwrap_or_else(|| empty_value.into()); + (crumbs.clone_ref(), expression) + })); + area_frp.source.on_port_code_update <+ code_update; } } - init_color.emit(()); - area_frp.set_view_mode.emit(area_frp.view_mode.value()); + port_shape.display_object().clone_ref() }; @@ -475,9 +560,51 @@ impl Model { } let new_parent_frp = Some(node.frp.output.clone_ref()); let new_shift = if !not_a_port { 0 } else { builder.shift + local_char_offset }; - builder.nested(new_parent, new_parent_frp, is_parensed, last_argument, new_shift) + builder.nested(new_parent, new_parent_frp, is_parensed, new_shift) }); *self.id_crumbs_map.borrow_mut() = id_crumbs_map; + *self.widgets_map.borrow_mut() = widgets_map; + area_frp.set_view_mode.emit(area_frp.view_mode.value()); + } + + fn init_port_widget( + &self, + port: &mut PortRefMut, + call_info: &CallInfoMap, + ) -> Option<(WidgetBind, widget::View)> { + let call_id = port.kind.call_id().filter(|id| call_info.has_target(id))?; + let argument_info = port.kind.argument_info()?; + let argument_name = argument_info.name.as_ref()?.clone(); + + let widget_bind = WidgetBind { call_id, argument_name }; + + + // Try getting the previous widget by exact target/argument ID first, which is + // necessary when the argument expression was replaced. This lookup can fail + // when the target expression was replaced, but the widget argument expression + // wasn't. In that case, try to reuse the widget from old argument node under + // the same ast ID. + let prev_widgets_map = self.widgets_map.borrow(); + let prev_id_crumbs_map = self.id_crumbs_map.borrow(); + let prev_crumbs = prev_widgets_map + .get(&widget_bind) + .or_else(|| port.ast_id.as_ref().and_then(|id| prev_id_crumbs_map.get(id))); + let prev_widget = prev_crumbs.and_then(|crumbs| { + let prev_expression = self.expression.borrow(); + let prev_root = prev_expression.span_tree.root_ref(); + let prev_node = prev_root.get_descendant(crumbs).ok()?; + let prev_widget = prev_node.payload.widget.as_ref()?.clone_ref(); + Some(prev_widget) + }); + + let widget = match prev_widget { + Some(prev_widget) => port.payload.use_existing_widget(prev_widget), + None => port.payload.init_widget(&self.app), + }; + + widget.set_node_data(widget::NodeData { argument_info, node_height: node::HEIGHT }); + + Some((widget_bind, widget)) } /// Initializes FRP network for every port. Please note that the networks are connected @@ -500,7 +627,6 @@ impl Model { // === Type Computation === - let parent_tp = parent_tp.clone().unwrap_or_else(|| { frp::extend! { port_network empty_parent_tp <- source::>(); @@ -660,11 +786,22 @@ impl Model { /// may require some edges to re-color, which consequently will require to checking the current /// expression types. #[profile(Debug)] - fn init_new_expression(&self, expression: Expression) { + fn init_new_expression( + &self, + expression: Expression, + area_frp: &FrpEndpoints, + call_info: &CallInfoMap, + ) { *self.expression.borrow_mut() = expression; let expression = self.expression.borrow(); expression.root_ref().dfs_with_layer_data((), |node, _| { node.frp.set_definition_type(node.tp().cloned().map(|t| t.into())); + let call_id = node.kind.call_id(); + let widget_request = + call_id.and_then(|call_id| Some((call_id, call_info.target(&call_id)?))); + if let Some(widget_request) = widget_request { + area_frp.source.requested_widgets.emit(widget_request); + } }); } @@ -679,15 +816,18 @@ impl Model { ) -> Expression { let mut new_expression = Expression::from(new_expression.into()); if DEBUG { - debug!("\n\n=====================\nSET EXPR: {}", new_expression.code) + debug!("set expression: \n{:?}", new_expression.tree_pretty_printer()); } + + let call_info = CallInfoMap::scan_expression(&new_expression); self.set_label_on_new_expression(&new_expression); - self.build_port_shapes_on_new_expression(&mut new_expression, area_frp); + self.build_port_shapes_on_new_expression(&mut new_expression, area_frp, &call_info); self.init_port_frp_on_new_expression(&mut new_expression, area_frp); - self.init_new_expression(new_expression.clone()); + self.init_new_expression(new_expression.clone(), area_frp, &call_info); if is_editing { self.label.set_cursor_at_text_end(); } + self.set_widgets_visibility(!is_editing); new_expression } } @@ -732,6 +872,9 @@ ensogl::define_endpoints! { /// colored if the definition type was present. set_expression_usage_type (Crumbs,Option), + /// Update widget metadata for widgets already present in this input area. + update_widgets (WidgetUpdates), + /// Enable / disable port hovering. The optional type indicates the type of the active edge /// if any. It is used to highlight ports if they are missing type information or if their /// types are polymorphic. @@ -739,6 +882,7 @@ ensogl::define_endpoints! { set_view_mode (view::Mode), set_profiling_status (profiling::Status), + } Output { @@ -754,6 +898,10 @@ ensogl::define_endpoints! { on_port_code_update (Crumbs,ImString), on_background_press (), view_mode (view::Mode), + /// A set of widgets attached to a method requires metadata to be queried. The tuple + /// contains the ID of the call expression the widget is attached to, and the ID of that + /// call's target expression (`self` or first argument). + requested_widgets (ast::Id, ast::Id), } } @@ -805,6 +953,7 @@ impl Area { eval frp.input.set_editing ([model](edit_mode) { model.label.deprecated_set_focus(edit_mode); + model.set_widgets_visibility(!edit_mode); if *edit_mode { // Reset the code to hide non-connected port names. model.label.set_content(model.expression.borrow().code.clone()); @@ -869,6 +1018,9 @@ impl Area { eval frp.set_expression_usage_type (((a,b)) model.set_expression_usage_type(a,b)); + // === Widgets === + + eval frp.update_widgets ((a) model.apply_widget_updates(a)); // === View Mode === @@ -920,7 +1072,7 @@ impl Area { expression.span_tree.root_ref().get_descendant(crumbs).ok().and_then(|t| t.tp.value()) } - /// A crumb by AST id. + /// A crumb by AST ID. pub fn get_crumbs_by_id(&self, id: ast::Id) -> Option { self.model.id_crumbs_map.borrow().get(&id).cloned() } @@ -944,19 +1096,17 @@ impl Area { /// parent layer when building the nested one. #[derive(Clone, Debug)] struct PortLayerBuilder { - parent_frp: Option, + parent_frp: Option, /// Parent port display object. - parent: display::object::Instance, + parent: display::object::Instance, /// Information whether the parent port was a parensed expression. - parent_parensed: bool, + parent_parensed: bool, /// The number of chars the expression should be shifted. For example, consider /// `(foo bar)`, where expression `foo bar` does not get its own port, and thus a 1 char /// shift should be applied when considering its children. - shift: usize, + shift: usize, /// The depth at which the current expression is, where root is at depth 0. - depth: usize, - /// The crumb of last child argument of parent node. Does not count insertion points. - parent_last_argument: Option, + depth: usize, } impl PortLayerBuilder { @@ -966,16 +1116,15 @@ impl PortLayerBuilder { parent: impl display::Object, parent_frp: Option, parent_parensed: bool, - parent_last_argument: Option, shift: usize, depth: usize, ) -> Self { let parent = parent.display_object().clone_ref(); - Self { parent_frp, parent, parent_parensed, shift, depth, parent_last_argument } + Self { parent_frp, parent, parent_parensed, shift, depth } } fn empty(parent: impl display::Object) -> Self { - Self::new(parent, default(), default(), default(), default(), default()) + Self::new(parent, default(), default(), default(), default()) } /// Create a nested builder with increased depth and updated `parent_frp`. @@ -985,12 +1134,11 @@ impl PortLayerBuilder { parent: display::object::Instance, new_parent_frp: Option, parent_parensed: bool, - parent_last_argument: Option, shift: usize, ) -> Self { let depth = self.depth + 1; let parent_frp = new_parent_frp.or_else(|| self.parent_frp.clone()); - Self::new(parent, parent_frp, parent_parensed, parent_last_argument, shift, depth) + Self::new(parent, parent_frp, parent_parensed, shift, depth) } } @@ -999,3 +1147,55 @@ impl display::Object for Area { &self.model.display_object } } + +/// =================== +/// === CallInfoMap === +/// =================== + +#[derive(Debug, Deref)] +struct CallInfoMap { + /// The map from node's call_id to call information. + call_info: HashMap, +} + +/// Information about the call expression, which are derived from the span tree. +#[derive(Debug, Default)] +struct CallInfo { + /// The AST ID associated with `This` span of the call expression. + target_id: Option, + /// The crumbs of last argument span associated with the call expression. + last_argument: Option, +} + +impl CallInfoMap { + fn scan_expression(expression: &SpanTree) -> Self { + let mut call_info: HashMap = HashMap::new(); + expression.root_ref().dfs(|node| { + if let Some(call_id) = node.kind.call_id() { + let mut entry = call_info.entry(call_id).or_default(); + if node.kind.is_this() { + entry.target_id = node.ast_id; + } else if node.kind.is_argument() { + entry.last_argument = Some(node.crumbs.clone()); + } + } + }); + + Self { call_info } + } + + fn has_target(&self, call_id: &ast::Id) -> bool { + self.call_info.get(call_id).map_or(false, |info| info.target_id.is_some()) + } + + fn target(&self, call_id: &ast::Id) -> Option { + self.call_info.get(call_id).and_then(|info| info.target_id) + } + + fn is_last_argument(&self, node: &PortRefMut) -> bool { + let call_id = node.kind.call_id(); + let info = call_id.and_then(|call_id| self.call_info.get(&call_id)); + let last_argument = info.and_then(|info| info.last_argument.as_ref()); + last_argument.map_or(false, |crumbs| crumbs == &node.crumbs) + } +} diff --git a/app/gui/view/graph-editor/src/component/node/input/port.rs b/app/gui/view/graph-editor/src/component/node/input/port.rs index 074cd135a961..accc7018d9c3 100644 --- a/app/gui/view/graph-editor/src/component/node/input/port.rs +++ b/app/gui/view/graph-editor/src/component/node/input/port.rs @@ -5,7 +5,7 @@ use enso_text::unit::*; use ensogl::display::shape::*; use crate::node::input::area; -use crate::node::input::widget::Widget; +use crate::node::input::widget; use crate::Type; use ensogl::application::Application; @@ -154,7 +154,7 @@ ensogl::define_endpoints! { pub struct Model { pub frp: Frp, pub shape: Option, - pub widget: Option, + pub widget: Option, pub index: ByteDiff, pub local_index: ByteDiff, pub length: ByteDiff, @@ -184,16 +184,18 @@ impl Model { self.shape.as_ref().unwrap().clone_ref() } - /// Widget initialization. Same rules apply as for the shape initialization. - pub fn init_widget( - &mut self, - app: &Application, - argument_info: Option, - node_height: f32, - ) -> Option { - let Some(argument_info) = argument_info else { return None }; - self.widget = Widget::new(app, argument_info, node_height); - self.widget.clone_ref() + /// Widget initialization. Only nodes that represent function arguments or argument placeholders + /// will have widgets created for them. + pub fn init_widget(&mut self, app: &Application) -> widget::View { + let widget = widget::View::new(app); + self.widget = Some(widget.clone_ref()); + widget + } + + /// Assign an existing widget to this port. + pub fn use_existing_widget(&mut self, widget: widget::View) -> widget::View { + self.widget = Some(widget.clone_ref()); + widget } /// The range of this port. diff --git a/app/gui/view/graph-editor/src/component/node/input/widget.rs b/app/gui/view/graph-editor/src/component/node/input/widget.rs index 0914fa66da39..be081326c258 100644 --- a/app/gui/view/graph-editor/src/component/node/input/widget.rs +++ b/app/gui/view/graph-editor/src/component/node/input/widget.rs @@ -8,73 +8,253 @@ use ensogl::data::color; use ensogl::display; use ensogl::display::object::event; use ensogl_component::drop_down::Dropdown; -use frp::stream::EventEmitter; -// ================== -// === NodeWidget === -// ================== +/// ================= +/// === Constants === +/// ================= + +const DOT_COLOR: color::Lch = color::Lch::new(0.56708, 0.23249, 0.71372); + + + +// =========== +// === FRP === +// =========== ensogl::define_endpoints_2! { Input { - set_current_value(Option), - set_focused(bool), + set_metadata (Option), + set_node_data (NodeData), + set_current_value (Option), + set_focused (bool), + set_visible (bool), } Output { value_changed(Option), } } -/// Possible widgets for a node input. -/// -/// Currently all widget types are hardcoded. This is likely to be a temporary solution. In the -/// future the set of widget types might be dynamic, similar to visualizations. -#[derive(Clone, Debug, CloneRef)] -pub enum Widget { - /// A widget for selecting a single value from a list of available options. - SingleChoice(SingleChoice), +/// Widget metadata that comes from an asynchronous visualization. Defines which widget should be +/// used and a set of options that it should allow to choose from. +#[derive(Debug, Clone)] +#[allow(missing_docs)] +pub struct Metadata { + pub kind: Kind, + pub display: Display, + pub dynamic_entries: Vec, +} + +/// Widget display mode. Determines when the widget should be expanded. +#[derive(serde::Deserialize, Debug, Clone, Copy, Default)] +pub enum Display { + /// The widget should always be in its expanded mode. + #[default] + Always, + /// The widget should only be in its expanded mode when it has non-default value. + #[serde(rename = "When_Modified")] + WhenModified, + /// The widget should only be in its expanded mode whe the whole node is expanded. + #[serde(rename = "Expanded_Only")] + ExpandedOnly, +} + +/// The data of node port that this widget is attached to. Available immediately after widget +/// creation. Can be updated later when the node data changes. +#[derive(Debug, Clone, Default)] +#[allow(missing_docs)] +pub struct NodeData { + pub argument_info: span_tree::ArgumentInfo, + pub node_height: f32, +} + + + +/// ================== +/// === SampledFrp === +/// ================== + +/// Sampled version of widget FRP endpoints that can be used by widget views that are initialized +/// on demand after first interaction. Without samplers, when a widget view would be initialized +/// after the endpoints were set, it would not receive previously set endpoint values. +#[derive(Debug, Clone, CloneRef)] +struct SampledFrp { + set_current_value: frp::Sampler>, + set_visible: frp::Sampler, + set_focused: frp::Sampler, + out_value_changed: frp::Any>, } -impl Widget { + + +// ============== +// === Widget === +// ============== + +/// The node widget view. Represents one widget of any kind on the node input area. Can change its +/// appearance and behavior depending on the widget metadata updates, without being recreated. +#[derive(Debug, Clone, CloneRef)] +pub struct View { + frp: Frp, + model: Rc, +} + +impl View { + /// Create a new node widget. The widget is initialized to empty state, waiting for widget + /// metadata to be provided using `set_node_data` and `set_metadata` FRP endpoints. + pub fn new(app: &Application) -> Self { + let frp = Frp::new(); + let model = Rc::new(Model::new(app)); + Self { frp, model }.init() + } + + /// Widget FRP API. Contains all endpoints that can be used to control the widget of any kind. + pub fn frp(&self) -> &Frp { + &self.frp + } + + fn init(self) -> Self { + let model = &self.model; + let frp = &self.frp; + let network = &frp.network; + let input = &frp.input; + + frp::extend! { network + widget_data <- all(&input.set_metadata, &input.set_node_data); + + set_current_value <- input.set_current_value.sampler(); + set_visible <- input.set_visible.sampler(); + set_focused <- input.set_focused.sampler(); + let out_value_changed = frp.private.output.value_changed.clone_ref(); + let sampled_frp = SampledFrp { set_current_value, set_visible, set_focused, out_value_changed }; + + eval widget_data([model, sampled_frp]((meta, node_data)) { + model.set_widget_data(&sampled_frp, meta, node_data); + }); + } + + self + } +} + + +/// ============= +/// === Model === +/// ============= + +#[derive(Debug)] +struct Model { + app: Application, + display_object: display::object::Instance, + kind_model: RefCell>, +} + +impl Model { /// Create a new node widget, selecting the appropriate widget type based on the provided /// argument info. - pub fn new( - app: &Application, - argument_info: span_tree::ArgumentInfo, - node_height: f32, - ) -> Option { - // TODO [PG] Support more widgets, use engine provided widget type. - // https://www.pivotaltracker.com/story/show/184012753 - if !argument_info.tag_values.is_empty() { - Some(Self::SingleChoice(SingleChoice::new(app, argument_info, node_height))) - } else { - None - } + fn new(app: &Application) -> Self { + let app = app.clone_ref(); + let display_object = display::object::Instance::new(); + let kind = default(); + Self { app, display_object, kind_model: kind } } - fn frp(&self) -> &Frp { - match self { - Self::SingleChoice(s) => &s.frp, + fn set_widget_data(&self, frp: &SampledFrp, meta: &Option, node_data: &NodeData) { + trace!("Setting widget data: {:?} {:?}", meta, node_data); + + let has_tag_values = !node_data.argument_info.tag_values.is_empty(); + let kind_fallback = has_tag_values.then_some(Kind::SingleChoice); + + let desired_kind = meta.as_ref().map(|m| m.kind).or(kind_fallback); + let current_kind = self.kind_model.borrow().as_ref().map(|m| m.kind()); + + if current_kind != desired_kind { + *self.kind_model.borrow_mut() = desired_kind.map(|desired_kind| { + KindModel::new(&self.app, &self.display_object, desired_kind, frp, meta, node_data) + }); + } else if let Some(model) = self.kind_model.borrow().as_ref() { + model.update(meta, node_data); } } } -impl Deref for Widget { +impl Deref for View { type Target = Frp; fn deref(&self) -> &Self::Target { self.frp() } } -impl display::Object for Widget { +impl display::Object for View { fn display_object(&self) -> &display::object::Instance { + &self.model.display_object + } +} + + + +// ======================== +// === KindModel / Kind === +// ======================== + +/// Possible widgets for a node input. +/// +/// Currently, all widget types are hardcoded. This is likely to be a temporary solution. In the +/// future the widget types might be user-defined, similar to visualizations. +#[derive(serde::Deserialize, Clone, Copy, Debug, PartialEq, Eq)] +pub enum Kind { + /// A widget for selecting a single value from a list of available options. + #[serde(rename = "Single_Choice")] + SingleChoice, +} + +/// A part of widget model that is dependant on the widget kind. +#[derive(Debug)] +pub enum KindModel { + /// A widget for selecting a single value from a list of available options. + SingleChoice(SingleChoiceModel), +} + +impl KindModel { + fn new( + app: &Application, + display_object: &display::object::Instance, + kind: Kind, + frp: &SampledFrp, + meta: &Option, + node_data: &NodeData, + ) -> Self { + let this = match kind { + Kind::SingleChoice => + Self::SingleChoice(SingleChoiceModel::new(app, display_object, frp)), + }; + + this.update(meta, node_data); + this + } + + fn update(&self, meta: &Option, node_data: &NodeData) { + match self { + KindModel::SingleChoice(inner) => { + let dynamic_entries = meta.as_ref().map(|meta| meta.dynamic_entries.clone()); + let tag_values = node_data.argument_info.tag_values.iter().map(Into::into); + let entries = dynamic_entries.unwrap_or_else(|| tag_values.collect()); + + inner.set_node_height(node_data.node_height); + inner.set_entries(entries); + } + } + } + fn kind(&self) -> Kind { match self { - Self::SingleChoice(s) => &s.display_object, + Self::SingleChoice(_) => Kind::SingleChoice, } } } + + // ================= // === Dot Shape === // ================= @@ -104,72 +284,71 @@ pub mod dot { /// A widget for selecting a single value from a list of available options. The options can be /// provided as a static list of strings from argument `tag_values`, or as a dynamic expression. -#[derive(Clone, Debug, CloneRef)] -pub struct SingleChoice { - display_object: display::object::Instance, - frp: Frp, +#[derive(Debug)] +pub struct SingleChoiceModel { #[allow(dead_code)] + network: frp::Network, dropdown: Rc>, /// temporary click handling activation_dot: dot::View, } -impl SingleChoice { - fn new(app: &Application, argument_info: span_tree::ArgumentInfo, node_height: f32) -> Self { - let display_object = display::object::Instance::new(); - let dropdown = app.new_view::>(); - display_object.add_child(&dropdown); - let frp = Frp::new(); - let network = &frp.network; - let input = &frp.input; - dropdown.set_y(-node_height); - dropdown.set_max_open_size(Vector2(300.0, 500.0)); - +impl SingleChoiceModel { + fn new( + app: &Application, + display_object: &display::object::Instance, + frp: &SampledFrp, + ) -> Self { let activation_dot = dot::View::new(); - let color: color::Rgba = color::Lcha::new(0.56708, 0.23249, 0.71372, 1.0).into(); - activation_dot.color.set(color.into()); activation_dot.set_size((15.0, 15.0)); - activation_dot.set_y(-node_height / 2.0); display_object.add_child(&activation_dot); - - let dropdown = if !argument_info.tag_values.is_empty() { - let entries: Vec = argument_info.tag_values.iter().map(Into::into).collect(); - let app = app.clone_ref(); - let display_object = display_object.clone_ref(); - let frp = frp.clone_ref(); - // Register a watcher for current value, to allow its value to be retrieved at any time - // when dropdown is initialized. - let current_value_watch = input.set_current_value.register_watch(); - let lazy = LazyDropdown::NotInitialized { - app, - display_object, - frp, - node_height, - entries, - _current_value: current_value_watch, - }; - Rc::new(RefCell::new(lazy)) - } else { - // TODO [PG]: Support dynamic entries. - // https://www.pivotaltracker.com/story/show/184012743 - unimplemented!(); + frp::new_network! { network + init <- source_(); + let focus_in = display_object.on_event::(); + let focus_out = display_object.on_event::(); + is_focused <- bool(&focus_out, &focus_in); + is_open <- frp.set_visible && is_focused; + is_open <- is_open.sampler(); }; + let set_current_value = frp.set_current_value.clone_ref(); + let dropdown_output = frp.out_value_changed.clone_ref(); + let dropdown = + LazyDropdown::new(app, display_object, set_current_value, is_open, dropdown_output); + let dropdown = Rc::new(RefCell::new(dropdown)); + frp::extend! { network let dot_clicked = activation_dot.events.mouse_down_primary.clone_ref(); toggle_focus <- dot_clicked.map(f!([display_object](()) !display_object.is_focused())); - set_focused <- any(toggle_focus, input.set_focused); + set_focused <- any(toggle_focus, frp.set_focused); eval set_focused([display_object](focus) match focus { true => display_object.focus(), false => display_object.blur(), }); - let focus_in = display_object.on_event::(); + set_visible <- all(&frp.set_visible, &init)._0(); + dot_alpha <- set_visible.map(|visible| if *visible { 1.0 } else { 0.0 }); + dot_color <- dot_alpha.map(|alpha| DOT_COLOR.with_alpha(*alpha)); + eval dot_color([activation_dot] (color) { + activation_dot.color.set(color::Rgba::from(color).into()); + }); + eval focus_in((_) dropdown.borrow_mut().initialize_on_open()); } - Self { display_object, frp, dropdown, activation_dot } + init.emit(()); + + Self { network, dropdown, activation_dot } + } + + fn set_node_height(&self, node_height: f32) { + self.activation_dot.set_y(-node_height / 2.0 - 1.0); + self.dropdown.borrow_mut().set_node_height(node_height); + } + + fn set_entries(&self, values: Vec) { + self.dropdown.borrow_mut().set_entries(values); } } @@ -190,23 +369,74 @@ impl SingleChoice { #[derive(Debug)] enum LazyDropdown { NotInitialized { - app: Application, - display_object: display::object::Instance, - frp: Frp, - node_height: f32, - entries: Vec, - _current_value: frp::data::watch::Handle, + app: Application, + display_object: display::object::Instance, + node_height: f32, + entries: Vec, + set_current_value: frp::Sampler>, + is_open: frp::Sampler, + output_value: frp::Any>, }, - Initialized(Dropdown), + Initialized(Dropdown, frp::Network), } impl LazyDropdown { + fn new( + app: &Application, + display_object: &display::object::Instance, + set_current_value: frp::Sampler>, + is_open: frp::Sampler, + output_value: frp::Any>, + ) -> Self { + let app = app.clone_ref(); + let display_object = display_object.clone_ref(); + let node_height = default(); + let entries = default(); + LazyDropdown::NotInitialized { + app, + display_object, + node_height, + entries, + set_current_value, + is_open, + output_value, + } + } + + fn set_node_height(&mut self, new_node_height: f32) { + match self { + LazyDropdown::Initialized(dropdown, ..) => { + dropdown.set_y(-new_node_height); + } + LazyDropdown::NotInitialized { node_height, .. } => { + *node_height = new_node_height; + } + } + } + + fn set_entries(&mut self, new_entries: Vec) { + match self { + LazyDropdown::Initialized(dropdown, ..) => { + dropdown.set_all_entries(new_entries); + } + LazyDropdown::NotInitialized { entries, .. } => { + *entries = new_entries; + } + } + } + fn initialize_on_open(&mut self) { match self { - LazyDropdown::Initialized(_) => {} + LazyDropdown::Initialized(..) => {} LazyDropdown::NotInitialized { - app, display_object, frp, node_height, entries, .. + app, + display_object, + node_height, + entries, + is_open, + set_current_value, + output_value, } => { let dropdown = app.new_view::>(); display_object.add_child(&dropdown); @@ -216,28 +446,19 @@ impl LazyDropdown { dropdown.set_all_entries(std::mem::take(entries)); dropdown.allow_deselect_all(true); - let network = &frp.network; - let input = &frp.input; - let output = &frp.private.output; - - frp::extend! { network + frp::new_network! { network init <- source_(); - current_value <- all(&input.set_current_value, &init)._0(); + current_value <- all(set_current_value, &init)._0(); dropdown.set_selected_entries <+ current_value.map(|s| s.iter().cloned().collect()); first_selected_entry <- dropdown.selected_entries.map(|e| e.iter().next().cloned()); - output.value_changed <+ first_selected_entry.on_change(); - - let focus_in = display_object.on_event::(); - let focus_out = display_object.on_event::(); - is_focused <- bool(&focus_out, &focus_in); + output_value <+ first_selected_entry.on_change(); - dropdown.set_open <+ is_focused; + is_open <- all(is_open, &init)._0(); + dropdown.set_open <+ is_open.on_change(); } init.emit(()); - dropdown.set_open(true); - - *self = LazyDropdown::Initialized(dropdown); + *self = LazyDropdown::Initialized(dropdown, network); } } } diff --git a/app/gui/view/graph-editor/src/lib.rs b/app/gui/view/graph-editor/src/lib.rs index fa72115bca56..59c7e9b8ffbf 100644 --- a/app/gui/view/graph-editor/src/lib.rs +++ b/app/gui/view/graph-editor/src/lib.rs @@ -536,11 +536,16 @@ ensogl::define_endpoints_2! { // === Visualization === - /// Simulates a visualization open press event. In case the event will be shortly followed by `release_visualization_visibility`, the visualization will be shown permanently. In other case, it will be disabled as soon as the `release_visualization_visibility` is emitted. + /// Simulates a visualization open press event. In case the event will be shortly followed + /// by `release_visualization_visibility`, the visualization will be shown permanently. In + /// other case, it will be disabled as soon as the `release_visualization_visibility` is + /// emitted. press_visualization_visibility(), - /// Simulates a visualization open double press event. This event toggles the visualization fullscreen mode. + /// Simulates a visualization open double press event. This event toggles the visualization + /// fullscreen mode. double_press_visualization_visibility(), - /// Simulates a visualization open release event. See `press_visualization_visibility` to learn more. + /// Simulates a visualization open release event. See `press_visualization_visibility` to + /// learn more. release_visualization_visibility(), /// Cycle the visualization for the selected nodes. cycle_visualization_for_selected_node(), @@ -570,7 +575,8 @@ ensogl::define_endpoints_2! { debug_push_breadcrumb(), /// Pop a breadcrumb without notifying the controller. debug_pop_breadcrumb(), - /// Set a test visualization data for the selected nodes. Useful for testing visualizations during their development. + /// Set a test visualization data for the selected nodes. Useful for testing visualizations + /// during their development. debug_set_test_visualization_data_for_selected_node(), @@ -602,7 +608,7 @@ ensogl::define_endpoints_2! { set_node_comment ((NodeId,node::Comment)), set_node_position ((NodeId,Vector2)), set_expression_usage_type ((NodeId,ast::Id,Option)), - set_method_pointer ((ast::Id,Option)), + update_node_widgets ((NodeId,WidgetUpdates)), cycle_visualization (NodeId), set_visualization ((NodeId,Option)), register_visualization (Option), @@ -622,6 +628,7 @@ ensogl::define_endpoints_2! { /// Drop an edge that is being dragged. drop_dragged_edge (), + } Output { @@ -701,6 +708,8 @@ ensogl::define_endpoints_2! { visualization_preprocessor_changed ((NodeId,PreprocessorConfiguration)), visualization_registry_reload_requested (), + widgets_requested (NodeId, ast::Id, ast::Id), + on_visualization_select (Switch), some_visualisation_selected (bool), @@ -968,9 +977,9 @@ pub struct LocalCall { -// ================== +// ==================== // === EdgeEndpoint === -// ================== +// ==================== #[derive(Clone, CloneRef, Debug, Default, Eq, PartialEq)] #[allow(missing_docs)] // FIXME[everyone] Public-facing API should be documented. @@ -1047,6 +1056,30 @@ impl Grid { +// ===================== +// === WidgetUpdates === +// ===================== + +/// A structure describing a widget update batch for arguments of single function call. +#[derive(Debug, Default, Clone)] +pub struct WidgetUpdates { + /// The function call expression ID. + pub call_id: ast::Id, + /// Update of a widget for each function argument. + pub updates: Rc>, +} + +/// A structure describing a widget update for specific argument of a function call. +#[derive(Debug)] +pub struct WidgetUpdate { + /// The function argument name that this widget is for. + pub argument_name: String, + /// Widget metadata queried from the language server. + pub meta: Option, +} + + + // ============= // === Nodes === // ============= @@ -1550,6 +1583,11 @@ impl GraphEditorModelWithNetwork { model.frp.private.output.node_expression_span_set.emit(args) }); + eval node.requested_widgets([model]((call_id, target_id)) { + let args = (node_id, *call_id, *target_id); + model.frp.private.output.widgets_requested.emit(args) + }); + // === Actions === @@ -2109,6 +2147,12 @@ impl GraphEditorModel { } } + fn update_node_widgets(&self, node_id: NodeId, updates: &WidgetUpdates) { + if let Some(node) = self.nodes.get_cloned_ref(&node_id) { + node.view.update_widgets.emit(updates.clone()); + } + } + fn disable_grid_snapping_for(&self, node_ids: &[NodeId]) { self.nodes.recompute_grid(node_ids.iter().cloned().collect()); } @@ -3240,7 +3284,7 @@ fn new_graph_editor(app: &Application) -> GraphEditor { nodes.get_cloned_ref(node_id).map(|node| node.all_edges()) )).unwrap(); eval edges_to_refresh ((edge) model.refresh_edge_position(*edge)); - + eval inputs.update_node_widgets(((node, updates)) model.update_node_widgets(*node, updates)); } @@ -3633,6 +3677,16 @@ fn new_graph_editor(app: &Application) -> GraphEditor { } + // ======================== + // === Focus management === + // ======================== + + frp::extend! { network + // Remove focus from any element when background is clicked. + eval_ touch.background.down (model.display_object.blur_tree()); + } + + // =============== // === Tooltip === // =============== diff --git a/engine/language-server/src/test/scala/org/enso/languageserver/websocket/json/VcsManagerTest.scala b/engine/language-server/src/test/scala/org/enso/languageserver/websocket/json/VcsManagerTest.scala index aeb6d4e5d820..b3e48751b1d5 100644 --- a/engine/language-server/src/test/scala/org/enso/languageserver/websocket/json/VcsManagerTest.scala +++ b/engine/language-server/src/test/scala/org/enso/languageserver/websocket/json/VcsManagerTest.scala @@ -2,7 +2,6 @@ package org.enso.languageserver.websocket.json import io.circe.literal._ import io.circe.parser.parse - import org.apache.commons.io.FileUtils import org.eclipse.jgit.api.{Git => JGit} import org.eclipse.jgit.lib.Repository @@ -11,7 +10,7 @@ import org.eclipse.jgit.storage.file.FileRepositoryBuilder import org.enso.languageserver.boot.ProfilingConfig import org.enso.languageserver.data._ import org.enso.languageserver.vcsmanager.VcsApi -import org.enso.testkit.RetrySpec +import org.enso.testkit.{FlakySpec, RetrySpec} import java.io.File import java.nio.charset.StandardCharsets @@ -20,7 +19,9 @@ import java.time.{Clock, LocalDate} import scala.concurrent.duration._ import scala.jdk.CollectionConverters._ -class VcsManagerTest extends BaseServerTest with RetrySpec { +// There is a race-condition in cleanup between individual tests casued by +// asynchronous initialization of JGit. Marking as Flaky until tests are re-done. +class VcsManagerTest extends BaseServerTest with RetrySpec with FlakySpec { override def mkConfig: Config = { val directoriesDir = Files.createTempDirectory(null).toRealPath() diff --git a/lib/rust/ensogl/component/text/src/font/glyph.rs b/lib/rust/ensogl/component/text/src/font/glyph.rs index b1e6da1b19b9..ad52152d6e39 100644 --- a/lib/rust/ensogl/component/text/src/font/glyph.rs +++ b/lib/rust/ensogl/component/text/src/font/glyph.rs @@ -549,6 +549,7 @@ fn get_context(scene: &Scene) -> Context { #[allow(missing_docs)] pub struct System { context: Context, + hinting: Immutable, pub font: FontWithAtlas, } @@ -559,8 +560,11 @@ impl System { let scene = scene.as_ref(); let fonts = scene.extension::(); let font = fonts.load(font_name); + let platform = platform::current(); + let hinting = HINTING_MAP.get(&(platform, font.name())).copied().unwrap_or_default(); + let hinting = Immutable(hinting); let context = get_context(scene); - Self { context, font } + Self { context, hinting, font } } /// Create new glyph. In the returned glyph the further parameters (position,size,character) @@ -579,13 +583,11 @@ impl System { let color_animation = color::Animation::new(frp.network()); let x_advance = default(); let attached_to_cursor = default(); - let platform = platform::current(); - let hinting = HINTING_MAP.get(&(platform, font.name())).copied().unwrap_or_default(); let view = glyph_shape::View::new_with_data(ShapeData { font }); view.color.set(Vector4::new(0.0, 0.0, 0.0, 0.0)); view.atlas_index.set(0.0); - view.opacity_increase.set(hinting.opacity_increase); - view.opacity_exponent.set(hinting.opacity_exponent); + view.opacity_increase.set(self.hinting.opacity_increase); + view.opacity_exponent.set(self.hinting.opacity_exponent); display_object.add_child(&view); let network = frp.network(); diff --git a/lib/rust/ensogl/core/src/display/object/instance.rs b/lib/rust/ensogl/core/src/display/object/instance.rs index 3b28fbf64ac0..12319c22bf62 100644 --- a/lib/rust/ensogl/core/src/display/object/instance.rs +++ b/lib/rust/ensogl/core/src/display/object/instance.rs @@ -1223,7 +1223,7 @@ impl WeakInstance { /// Checks whether this weak instance still exists (its strong instance was not dropped yet). pub fn exists(&self) -> bool { - self.upgrade().is_some() + self.weak.strong_count() > 0 } } @@ -1237,7 +1237,15 @@ impl InstanceDef { impl PartialEq for WeakInstance { fn eq(&self, other: &Self) -> bool { - self.exists() && other.exists() && self.weak.ptr_eq(&other.weak) + self.weak.ptr_eq(&other.weak) + } +} + +impl Eq for WeakInstance {} + +impl Hash for WeakInstance { + fn hash(&self, state: &mut H) { + self.weak.as_ptr().hash(state) } } @@ -1392,7 +1400,7 @@ pub mod dirty { type NewParent = crate::data::dirty::RefCellBool<()>; type ModifiedChildren = crate::data::dirty::RefCellSet; - type RemovedChildren = crate::data::dirty::RefCellVector; + type RemovedChildren = crate::data::dirty::RefCellSet; type Transformation = crate::data::dirty::RefCellBool; type SceneLayer = crate::data::dirty::RefCellBool; diff --git a/lib/rust/ensogl/core/src/system/gpu/data/attribute.rs b/lib/rust/ensogl/core/src/system/gpu/data/attribute.rs index e826f086b230..82d549660185 100644 --- a/lib/rust/ensogl/core/src/system/gpu/data/attribute.rs +++ b/lib/rust/ensogl/core/src/system/gpu/data/attribute.rs @@ -149,11 +149,8 @@ impl { pub fn add_instance(&mut self) -> InstanceIndex { let instance_count = 1; debug_span!("Adding {instance_count} instance(s).").in_scope(|| { - match self.free_ids.iter().next().copied() { - Some(ix) => { - self.free_ids.remove(&ix); - ix - } + match self.free_ids.pop_first() { + Some(ix) => ix, None => { let ix = self.size; self.size += instance_count; diff --git a/lib/rust/frp/src/network.rs b/lib/rust/frp/src/network.rs index 90646c481b61..f4fef7199e84 100644 --- a/lib/rust/frp/src/network.rs +++ b/lib/rust/frp/src/network.rs @@ -94,9 +94,14 @@ impl Network { WeakNetwork { data: Rc::downgrade(&self.data) } } + /// Get the number of strong references to this network. + pub fn strong_count(&self) -> usize { + Rc::strong_count(&self.data) + } + /// ID getter of this network. pub fn id(&self) -> NetworkId { - self.downgrade().id() + NetworkId(Rc::as_ptr(&self.data) as *const () as usize) } /// Store arbitrary item in this network. Used as a convenient storage of data associated with diff --git a/lib/rust/frp/src/nodes.rs b/lib/rust/frp/src/nodes.rs index 2492c2c3c897..6dd1adde4b58 100644 --- a/lib/rust/frp/src/nodes.rs +++ b/lib/rust/frp/src/nodes.rs @@ -2497,7 +2497,7 @@ where generics::ItemAt0: Data, { fn on_event(&self, stack: CallStack, event: &Output) { - self.emit_event(stack, (*event)._0()) + self.emit_event(stack, event._0()) } } @@ -2553,7 +2553,7 @@ where generics::ItemAt1: Data, { fn on_event(&self, stack: CallStack, event: &Output) { - self.emit_event(stack, (*event)._1()) + self.emit_event(stack, event._1()) } } @@ -2609,7 +2609,7 @@ where generics::ItemAt2: Data, { fn on_event(&self, stack: CallStack, event: &Output) { - self.emit_event(stack, (*event)._2()) + self.emit_event(stack, event._2()) } } diff --git a/lib/rust/prelude/src/debug.rs b/lib/rust/prelude/src/debug.rs index 0647d1657aa6..a8487fc0ba9a 100644 --- a/lib/rust/prelude/src/debug.rs +++ b/lib/rust/prelude/src/debug.rs @@ -77,12 +77,16 @@ fn next_clone_id() -> u64 { impl TraceCopies { /// Create enabled structure with appointed entity name (shared between all copies). pub fn enabled(name: impl Into) -> Self { - Self { clone_id: default(), handle: Rc::new(RefCell::new(Some(name.into()))) } + let this: Self = default(); + this.enable(name); + this } /// Assign a name to the entity (shared between all copies) and start printing logs. pub fn enable(&self, name: impl Into) { - *self.handle.borrow_mut() = Some(name.into()); + let name = name.into(); + debug!("[{name}] TraceCopies enabled"); + *self.handle.borrow_mut() = Some(name); } } @@ -93,7 +97,7 @@ impl Clone for TraceCopies { let handle = self.handle.clone(); if let Some(name) = &*borrow { let bt = backtrace(); - println!("[{name}] Cloning {} -> {clone_id} {bt}", self.clone_id); + debug!("[{name}] Cloning {} -> {clone_id} {bt}", self.clone_id); } Self { clone_id, handle } }