diff --git a/crates/calc/src/eval.rs b/crates/calc/src/eval.rs index 92a0a39fd..e640c63e1 100644 --- a/crates/calc/src/eval.rs +++ b/crates/calc/src/eval.rs @@ -247,7 +247,7 @@ impl<'h> Evaluator<'h> { if let Some(def) = wb.name(bare, scope_sheet) { let text = def.formula.clone(); let scope = def.scope.unwrap_or(self.sheet); - return match gridcraft_formula::parse(&text) { + return match gridcraft_formula::parse_name(&text) { Ok(expr) => { // Relative references in a name are relative to A1: used here, they move by // this cell's offset from it. diff --git a/crates/calc/src/tests.rs b/crates/calc/src/tests.rs index 513340983..5e62c29ea 100644 --- a/crates/calc/src/tests.rs +++ b/crates/calc/src/tests.rs @@ -515,6 +515,24 @@ fn names_and_sheets() { assert_eq!(t.num("A1"), 5.0); } +#[test] +fn name_referring_to_several_ranges() { + let mut t = T::new(); + // As Excel writes it in ``: a union with no parentheses around it. + let union = "Sheet1!$A$1:$A$2,Sheet1!$C$1:$C$2"; + t.wb.names.push(gridcraft_model::DefinedName { name: "u".into(), scope: None, formula: union.into(), comment: String::new(), hidden: false }); + for (at, v) in [("A1", "1"), ("A2", "2"), ("C1", "5"), ("C2", "7")] { + t.set(at, v); + } + t.set("E1", "=SUM(u)"); + t.set("E2", "=AREAS(u)"); + t.set("E3", "=MIN(u)"); + t.set("E4", "=INDEX(u,2,1,2)"); + assert_eq!((t.num("E1"), t.num("E2"), t.num("E3"), t.num("E4")), (15.0, 2.0, 1.0, 7.0)); + t.set("C2", "70"); + assert_eq!(t.num("E1"), 78.0); +} + #[test] fn legacy_array_formulas_fill_their_range() { let mut t = T::new(); diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 3da230b3b..7038bdb7a 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -1206,7 +1206,7 @@ pub(crate) fn rewrite_all_formulas(wb: &mut gridcraft_model::Workbook, target: & } } for n in wb.names.iter_mut() { - if let Ok(expr) = gridcraft_formula::parse(&n.formula) { + if let Ok(expr) = gridcraft_formula::parse_name(&n.formula) { // Names are workbook-level: unqualified refs don't occur; qualified ones adjust. let new = gridcraft_formula::adjust::adjust(expr, "\u{0}", target, e); n.formula = gridcraft_formula::print(&new); diff --git a/crates/engine/src/cmd/formulas.rs b/crates/engine/src/cmd/formulas.rs index 40df72df0..09a0a4644 100644 --- a/crates/engine/src/cmd/formulas.rs +++ b/crates/engine/src/cmd/formulas.rs @@ -265,7 +265,7 @@ fn define_name(s: &mut Session, p: &Json) -> Result { let refers = match str_param(p, "refersTo") { // As in Excel, relative references are relative to the active cell; names keep them // relative to A1, as files do. - Some(r) => match gridcraft_formula::parse(r.trim_start_matches('=')) { + Some(r) => match gridcraft_formula::parse_name(r.trim_start_matches('=')) { Ok(e) if gridcraft_formula::adjust::has_relative_refs(&e) => { let at = d.selection.active; gridcraft_formula::print(&gridcraft_formula::adjust::shift_relative_wrapping(e, -(at.row as i64), -(at.col as i64))) @@ -280,7 +280,7 @@ fn define_name(s: &mut Session, p: &Json) -> Result { format!("{}!{}", gridcraft_formula::quote_sheet(&sh.name), body) } }; - if gridcraft_formula::parse(&refers).is_err() { + if gridcraft_formula::parse_name(&refers).is_err() { return Err(EngineError::Other("There's a problem with this formula.".into())); } let comment = str_param(p, "comment").unwrap_or("").to_string(); @@ -355,7 +355,7 @@ fn rename_name_in_formulas(wb: &mut gridcraft_model::Workbook, old: &str, old_sc } for n in wb.names.iter_mut() { if n.scope.is_none_or(applies) - && let Some(e) = gridcraft_formula::parse(&n.formula).ok().and_then(rename) + && let Some(e) = gridcraft_formula::parse_name(&n.formula).ok().and_then(rename) { n.formula = gridcraft_formula::print(&e); } @@ -610,4 +610,27 @@ mod tests { let r = s.execute("formulas.defineName", json!({"name": "First", "refersTo": "=Sheet1!$A$1"})).unwrap(); assert_eq!(r["refersTo"], "=Sheet1!$A$1"); } + + #[test] + fn names_referring_to_several_ranges() { + let mut s = Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A1:C2", "values": [[1, 0, 5], [2, 0, 7]]})).unwrap(); + // Excel writes and accepts a union in a name without parentheses. + let r = s.execute("formulas.defineName", json!({"name": "u", "refersTo": "=Sheet1!$A$1:$A$2,Sheet1!$C$1:$C$2"})).unwrap(); + assert_eq!(r["refersTo"], "=Sheet1!$A$1:$A$2,Sheet1!$C$1:$C$2"); + for (cell, input) in [("E1", "=SUM(u)"), ("E2", "=AREAS(u)"), ("E3", "=MIN(u)"), ("E4", "=INDEX(u,2,1,2)")] { + s.execute("cell.set", json!({"cell": cell, "input": input})).unwrap(); + } + let v = |s: &mut Session, a: &str| s.execute("cell.get", json!({"cell": a})).unwrap()["value"].clone(); + assert_eq!(v(&mut s, "E1"), json!(15.0)); + assert_eq!(v(&mut s, "E2"), json!(2.0)); + assert_eq!(v(&mut s, "E3"), json!(1.0)); + assert_eq!(v(&mut s, "E4"), json!(7.0)); + // Renaming the sheet follows every area, and the definition keeps Excel's form. + s.execute("sheet.rename", json!({"name": "Data"})).unwrap(); + let name = s.doc().unwrap().wb.names.iter().find(|n| n.name == "u").map(|n| n.formula.clone()); + assert_eq!(name.as_deref(), Some("Data!$A$1:$A$2,Data!$C$1:$C$2")); + assert_eq!(v(&mut s, "E1"), json!(15.0)); + } } diff --git a/crates/engine/src/cmd/sheet.rs b/crates/engine/src/cmd/sheet.rs index f05b77d91..c30642e27 100644 --- a/crates/engine/src/cmd/sheet.rs +++ b/crates/engine/src/cmd/sheet.rs @@ -704,7 +704,7 @@ fn delete_sheet(s: &mut Session, p: &Json) -> Result { *sc -= 1; } // Names referring to the sheet become #REF!. - if let Ok(e) = gridcraft_formula::parse(&n.formula) { + if let Ok(e) = gridcraft_formula::parse_name(&n.formula) { let ne = gridcraft_formula::adjust::delete_sheet(e.clone(), &name, &order); if ne != e { n.formula = gridcraft_formula::print(&ne); @@ -766,7 +766,7 @@ fn rename_sheet(s: &mut Session, p: &Json) -> Result { } } for n in cx.wb.names.iter_mut() { - if let Ok(e) = gridcraft_formula::parse(&n.formula) { + if let Ok(e) = gridcraft_formula::parse_name(&n.formula) { n.formula = gridcraft_formula::print(&gridcraft_formula::adjust::rename_sheet(e, &old, &name)); } } @@ -805,7 +805,7 @@ fn move_sheet(s: &mut Session, p: &Json) -> Result { .iter() .filter(|nm| nm.scope == Some(i)) .map(|nm| { - let formula = gridcraft_formula::parse(&nm.formula) + let formula = gridcraft_formula::parse_name(&nm.formula) .map(|e| gridcraft_formula::print(&gridcraft_formula::adjust::rename_sheet(e, &base, &sh.name))) .unwrap_or_else(|_| nm.formula.clone()); gridcraft_model::DefinedName { scope: Some(at), formula, ..nm.clone() } diff --git a/crates/formula/src/lib.rs b/crates/formula/src/lib.rs index e193c3fb4..991db2ef8 100644 --- a/crates/formula/src/lib.rs +++ b/crates/formula/src/lib.rs @@ -12,7 +12,7 @@ pub mod printer; pub use ast::{Anchor, BinOp, Expr, RefKind, Reference, SheetSel, StructItem, StructRef, TrimMode, UnOp}; pub use input::{FormulaLocale, parse_input, print_input}; -pub use parser::{ParseError, normalize_function_name, parse}; +pub use parser::{ParseError, normalize_function_name, parse, parse_name}; pub use printer::{print, print_r1c1, quote_sheet}; #[cfg(test)] diff --git a/crates/formula/src/parser.rs b/crates/formula/src/parser.rs index 4fd3b7bb2..ee561bb53 100644 --- a/crates/formula/src/parser.rs +++ b/crates/formula/src/parser.rs @@ -23,12 +23,22 @@ pub fn parse(src: &str) -> Result { parse_syntax(src, crate::FormulaLocale::En) } +/// Parses a defined name's formula. Unlike a cell formula, a comma at the top level is the +/// union operator: Excel stores and shows a name over several ranges as `Sheet1!$A$1,Sheet1!$C$1`. +pub fn parse_name(src: &str) -> Result { + parse_top(src, crate::FormulaLocale::En, true) +} + pub(crate) fn parse_syntax(src: &str, locale: crate::FormulaLocale) -> Result { + parse_top(src, locale, false) +} + +fn parse_top(src: &str, locale: crate::FormulaLocale, allow_union: bool) -> Result { let body = src.strip_prefix('=').unwrap_or(src); let toks = tokenize_locale(body, locale).map_err(|e| ParseError { msg: e.msg, pos: e.pos })?; let toks = significant_spaces(toks); let mut p = Parser { toks, i: 0, depth: 0, input: locale == crate::FormulaLocale::Es }; - let e = p.expr(0, false)?; + let e = p.expr(0, allow_union)?; match p.peek() { Tok::Eof => Ok(e), t => Err(p.err(format!("unexpected {}", describe(t)))), diff --git a/crates/model/src/workbook.rs b/crates/model/src/workbook.rs index 18e197194..b66ec8f0e 100644 --- a/crates/model/src/workbook.rs +++ b/crates/model/src/workbook.rs @@ -228,7 +228,7 @@ impl Workbook { } } for n in self.names.iter_mut() { - if let Ok(e) = gridcraft_formula::parse(&n.formula) { + if let Ok(e) = gridcraft_formula::parse_name(&n.formula) { let ne = spell_calls(e.clone(), &lambdas); if ne != e { n.formula = gridcraft_formula::print(&ne); diff --git a/crates/xlsx/src/array_formula.rs b/crates/xlsx/src/array_formula.rs index caa5e8e9f..b2c09a749 100644 --- a/crates/xlsx/src/array_formula.rs +++ b/crates/xlsx/src/array_formula.rs @@ -57,7 +57,7 @@ impl Scan<'_> { None => (sheet, n.as_str()), }; if let Some(def) = self.wb.name(bare, scope) { - return match gridcraft_formula::parse(def.formula.trim_start_matches('=')) { + return match gridcraft_formula::parse_name(def.formula.trim_start_matches('=')) { Ok(expr) => self.expr(&expr, def.scope.unwrap_or(scope), array_context, depth + 1), Err(_) => Shape::array(), };