Rework the 'Math' expression family as 'Math Function' / 'Math Formula', and add 'Math Running' / 'Math Aggregate'

.branding

@@ -1,2 +1,2 @@
-https://github.com/Keavon/graphite-branded-assets/archive/4e42efe8dbf2ae2d82e12123aa9c103d5740a465.tar.gz
-6b7883a26952c77aad56758a11207761c2fcd1b61bf6dd5996c3cf9d4eb1cb5e
+https://github.com/Keavon/graphite-branded-assets/archive/70b260c21afad29140bbc63ddab952fec3dd024c.tar.gz
+5b05e7f4449b5f962755542e9b156891bbf2e0a87b7fc90798b8352d9225757d

document/migrations/nodes/src/lib.rs

@@ -2,4 +2,5 @@
 //! `cargo run migration new <identifier>` after bumping a node's `version(N)`.
 
 mod graphene_core;
+mod math_nodes;
 mod raster_nodes;

document/migrations/nodes/src/math_nodes/math_formula_from_math_f.rs

@@ -0,0 +1,107 @@
+use migration_core::{Node, Target, decl, literal_of, migration};
+use serde::{Deserialize, Serialize};
+
+const EXPRESSION: usize = 1;
+
+/// Frozen mirror of the `TaggedValue` variant this migration reads.
+#[derive(Serialize, Deserialize)]
+enum Tagged {
+	String(String),
+}
+
+/// The lone operators 'Math f(…)' applied across its items, in every spelling its classifier read when the node was split.
+const OPERATORS: &[&str] = &["+", "-", "*", "/", "^", "&&", "∧", "||", "∨", "<", "<=", "≤", ">", ">=", "≥", "==", "!=", "≠"];
+
+/// The variadic functions 'Math f(…)' called with all its items when given a lone name, also spelled with a leading `\`.
+const FUNCTIONS: &[&str] = &[
+	"hypot",
+	"min",
+	"max",
+	"mean",
+	"median",
+	"variance",
+	"variancepop",
+	"stdev",
+	"stdevpop",
+	"geomean",
+	"harmmean",
+	"rms",
+	"mode",
+	"count",
+	"xor",
+	"gcd",
+	"lcm",
+];
+
+/// 'Math f(…)' split by the kind of string it held: a lone operator or function name, which it applied across its items, moves to
+/// 'Math Aggregate', and an expression stays as 'Math Formula'. A wired string can't be read before it runs, so it stays an expression.
+#[migration("math_nodes::MathFNode"@1 => "math_nodes::MathFormulaNode"@2)]
+pub fn math_formula_from_math_f(node: &Node, target: &mut Target) {
+	let Some(slot) = node.inputs().get(EXPRESSION) else { return };
+	let Some(Tagged::String(expression)) = literal_of::<Tagged>(&slot.input) else { return };
+
+	let token = expression.trim();
+	let function = token.strip_prefix('\\').unwrap_or(token);
+	if OPERATORS.contains(&token) || FUNCTIONS.contains(&function) {
+		target.set(decl!("math_nodes::MathAggregateNode"@1));
+	}
+}
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+	use migration_core::{Declaration, NodeInput, literal};
+	use migration_runner::testing::{Fixture, SourceContract, check_commutation, check_undo};
+
+	const MATH_F: Declaration = decl!("math_nodes::MathFNode"@1);
+
+	fn contract() -> SourceContract {
+		// Only the items can read another 'Math f(…)', since the expression and output type don't take its number
+		SourceContract {
+			reads_own_output: |slot| slot == 0,
+			..SourceContract::new(3, |random, slot| match slot {
+				EXPRESSION => literal(Tagged::String(["+", "a * b", "\\max", " ≤ ", "min(a, b)"][random.below(5)].to_string())),
+				_ => literal(random.below(4) as f64),
+			})
+		}
+	}
+
+	fn migrated_declaration(expression: NodeInput) -> Option<Declaration> {
+		let mut fixture = Fixture::new(&MathFormulaFromMathF);
+		let anchor = fixture.add(MATH_F, vec![literal(1.), expression, literal(0.)]);
+		let migrated = fixture.migrate(&MathFormulaFromMathF).unwrap();
+		fixture.declaration_of(&migrated, anchor)
+	}
+
+	#[test]
+	fn rewrites_history() {
+		check_commutation(&MathFormulaFromMathF, &contract());
+		check_undo(&MathFormulaFromMathF, &contract());
+	}
+
+	#[test]
+	fn lone_operators_and_functions_become_aggregates() {
+		let aggregate = Some(decl!("math_nodes::MathAggregateNode"@1));
+		let formula = Some(decl!("math_nodes::MathFormulaNode"@2));
+		let expression = |text: &str| literal(Tagged::String(text.to_string()));
+
+		assert_eq!(migrated_declaration(expression("+")), aggregate);
+		assert_eq!(migrated_declaration(expression(" ≠ ")), aggregate);
+		assert_eq!(migrated_declaration(expression("max")), aggregate);
+		assert_eq!(migrated_declaration(expression("\\mean")), aggregate);
+		assert_eq!(migrated_declaration(expression("a + b")), formula);
+		assert_eq!(migrated_declaration(expression("max(a, b)")), formula);
+		assert_eq!(migrated_declaration(expression("")), formula);
+	}
+
+	#[test]
+	fn a_wired_expression_becomes_a_formula() {
+		let mut fixture = Fixture::new(&MathFormulaFromMathF);
+		let source = fixture.add(MATH_F, vec![literal(1.), literal(Tagged::String("+".to_string())), literal(0.)]);
+		let anchor = fixture.add(MATH_F, vec![literal(1.), NodeInput::Node { id: source, index: 0 }, literal(0.)]);
+		let migrated = fixture.migrate(&MathFormulaFromMathF).unwrap();
+
+		assert_eq!(fixture.declaration_of(&migrated, anchor), Some(decl!("math_nodes::MathFormulaNode"@2)));
+		assert_eq!(fixture.declaration_of(&migrated, source), Some(decl!("math_nodes::MathAggregateNode"@1)));
+	}
+}

document/migrations/nodes/src/math_nodes/mod.rs

@@ -0,0 +1,4 @@
+//! Migrations of the nodes in `math_nodes`.
+
+mod math_formula_from_math_f;
+mod renames;

document/migrations/nodes/src/math_nodes/renames.rs

@@ -0,0 +1,5 @@
+//! Nodes that only moved to a new identifier, or changed version with stored nodes staying as they are.
+
+use migration_core::rename;
+
+rename!("math_nodes::MathFxNode"@1 => "math_nodes::MathFunctionNode"@2);

editor/src/messages/layout/layout_message_handler.rs

@@ -118,7 +118,7 @@ impl MessageHandler<LayoutMessage, LayoutMessageContext<'_>> for LayoutMessageHa
 				};
 
 				// The reply carries the source it read, so the frontend can drop one that a later keystroke has outdated
-				let MathExpressionTokens { tokens, tooltips } = math_expression_tokens(&source, input.accepts_reducers);
+				let MathExpressionTokens { tokens, tooltips } = math_expression_tokens(&source, input.operator_only);
 				let completions = caret.map(|caret| math_expression_completions(&source, caret, asked));
 				responses.add(FrontendMessage::UpdateMathExpressionAnalysis {
 					widget_id,
@@ -371,9 +371,9 @@ impl LayoutMessageHandler {
 						};
 
 						// The widget is resent as stored, so its tokens and errors must be for its new value rather than its old one
-						let accepts_reducers = math_expression_input.accepts_reducers;
-						let error = math_expression_error(update_value, accepts_reducers);
-						let MathExpressionTokens { tokens, tooltips } = math_expression_tokens(update_value, accepts_reducers);
+						let operator_only = math_expression_input.operator_only;
+						let error = math_expression_error(update_value, operator_only);
+						let MathExpressionTokens { tokens, tooltips } = math_expression_tokens(update_value, operator_only);
 						math_expression_input.tokens = tokens;
 						math_expression_input.tooltips = tooltips;
 						math_expression_input.errors = math_expression_error_ranges(update_value, error.as_ref());

editor/src/messages/layout/utility_types/math_expression.rs

@@ -16,13 +16,16 @@ fn utf16_offsets(source: &str) -> Vec<u32> {
 	units
 }
 
-/// Why a math expression fails to parse, or `None` when it parses. Where the node applies a lone operator or function name like
-/// `+` or `min` across its items, that counts as valid too.
-pub fn math_expression_error(expression: &str, accepts_reducers: bool) -> Option<ParseError> {
+/// Why a math expression fails to parse, or `None` when it parses. Where the node takes only a lone operator or function name like
+/// `+` or `min` to apply across its items, anything else is the error instead.
+pub fn math_expression_error(expression: &str, operator_only: bool) -> Option<ParseError> {
 	// A blank expression is unfinished rather than wrong, so it goes unflagged like any empty field
-	if expression.trim().is_empty() || (accepts_reducers && math_parser::reducer::classify_reducer(expression, math_parser::context::NothingMap).is_some()) {
+	if expression.trim().is_empty() {
 		return None;
 	}
+	if operator_only {
+		return math_parser::reducer::reducer_error(expression);
+	}
 	math_parser::ast::Node::try_parse_from_str(expression).err()
 }
 
@@ -143,11 +146,11 @@ pub struct MathExpressionTokens {
 }
 
 /// The tokens of a math expression for the frontend to color and typeset, with the parser's byte offsets converted to code units.
-pub fn math_expression_tokens(source: &str, accepts_reducers: bool) -> MathExpressionTokens {
+pub fn math_expression_tokens(source: &str, operator_only: bool) -> MathExpressionTokens {
 	let units = utf16_offsets(source);
 	let mut tooltips: Vec<MathExpressionTooltip> = Vec::new();
 
-	let tokens = math_parser::highlight::highlight(source, accepts_reducers).into_iter().map(|token| {
+	let tokens = math_parser::highlight::highlight(source, operator_only).into_iter().map(|token| {
 		let tooltip = token.tooltip.map(|tooltip| MathExpressionTooltip {
 			label: tooltip.label,
 			form: tooltip.form,

editor/src/messages/layout/utility_types/widgets/input_widgets.rs

@@ -502,9 +502,9 @@ pub struct MathExpressionInput {
 	pub tooltips: Vec<MathExpressionTooltip>,
 	/// The stretches of the value its parse errors point at, which the frontend underlines.
 	pub errors: Vec<MathExpressionRange>,
-	/// Whether a lone operator or function name like `+` or `max` is an expression, applied across a list as by the Math node.
+	/// Whether the value is a lone operator or function name like `+` or `max`, applied across a list, rather than an expression.
 	#[serde(skip)]
-	pub accepts_reducers: bool,
+	pub operator_only: bool,
 	pub disabled: bool,
 
 	// Tooltips

editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs

@@ -1394,8 +1394,8 @@ fn static_input_inspector() -> InputInspector {
 		}),
 	);
 	map.insert(
-		// Like `math_expression`, where a lone reducer token like `+` or `min` applied across the node's items is also valid
-		"math_expression_or_reducer".to_string(),
+		// Like `math_expression`, but taking only a lone operator or function name like `+` or `min` to apply across the node's items
+		"math_operator".to_string(),
 		Box::new(|node_id, index, context| {
 			let info = ParameterWidgetsInfo::at_index(node_id, index, true, context);
 			Ok(vec![LayoutGroup::row(node_inspector::math_expression_widget(info, true))])

editor/src/messages/portfolio/document/node_graph/node_inspector.rs

@@ -481,16 +481,16 @@ pub fn text_widget(parameter_widgets_info: ParameterWidgetsInfo) -> Vec<WidgetIn
 }
 
 /// A math expression's typeset text field, with a warning icon in the assist area giving the error in its tooltip while the expression fails to parse.
-pub fn math_expression_widget(mut parameter_widgets_info: ParameterWidgetsInfo, accepts_reducers: bool) -> Vec<WidgetInstance> {
+pub fn math_expression_widget(mut parameter_widgets_info: ParameterWidgetsInfo, operator_only: bool) -> Vec<WidgetInstance> {
 	let Some(TaggedValue::String(expression)) = parameter_widgets_info.input().and_then(|input| input.as_non_exposed_value()) else {
 		return text_widget(parameter_widgets_info);
 	};
-	let error = math_expression_error(expression, accepts_reducers);
-	let MathExpressionTokens { tokens, tooltips } = math_expression_tokens(expression, accepts_reducers);
+	let error = math_expression_error(expression, operator_only);
+	let MathExpressionTokens { tokens, tooltips } = math_expression_tokens(expression, operator_only);
 	let input = MathExpressionInput::new(expression.clone())
 		.tokens(tokens)
 		.tooltips(tooltips)
-		.accepts_reducers(accepts_reducers)
+		.operator_only(operator_only)
 		.errors(math_expression_error_ranges(expression, error.as_ref()))
 		.on_update(parameter_widgets_info.update_value(|x: &MathExpressionInput| TaggedValue::String(x.value.clone())))
 		.on_commit(commit_value)
@@ -500,7 +500,7 @@ pub fn math_expression_widget(mut parameter_widgets_info: ParameterWidgetsInfo,
 	// padded to a checkbox's 16px so the field lines up with the rows around it
 	let icon = match &error {
 		Some(error) => IconLabel::new("Warning")
-			.tooltip_label("Invalid Math Expression")
+			.tooltip_label(if operator_only { "Invalid Operator" } else { "Invalid Math Expression" })
 			.tooltip_description(math_expression_error_description(error)),
 		None => IconLabel::new("Empty12px"),
 	};
@@ -3942,12 +3942,13 @@ mod tests {
 
 	#[test]
 	fn math_expressions_are_flagged_only_when_they_fail_to_parse() {
-		for (expression, accepts_reducers) in [("x * 2", false), ("", false), ("  ", true), ("min", false), ("+", true), ("min", true)] {
-			assert!(math_expression_error(expression, accepts_reducers).is_none(), "`{expression}`");
+		for (expression, operator_only) in [("x * 2", false), ("", false), ("  ", true), ("min", false), ("+", true), ("min", true), (" ≤ ", true)] {
+			assert!(math_expression_error(expression, operator_only).is_none(), "`{expression}`");
 		}
 
 		assert!(math_expression_error("2 +", false).is_some());
-		assert!(math_expression_error("+", false).is_some(), "a lone reducer is an error where the node applies none");
+		assert!(math_expression_error("+", false).is_some(), "a lone operator is an error where the node takes an expression");
+		assert!(math_expression_error("a + b", true).is_some(), "an expression is an error where the node takes a lone operator");
 		assert_eq!(description("sin(I)").as_deref(), Some("A matrix stands where a value is needed"));
 	}