diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index cc994f00f..496953104 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -148,6 +148,7 @@ pub(crate) fn input_to_cell(input: &str, old: Option<&Cell>, wb: &mut gridcraft_ parsed = gridcraft_formula::parse(&text); } let expr = parsed.map_err(|e| EngineError::Other(format!("There's a problem with this formula: {e}")))?; + let expr = wb.name_call_case(expr); let mut cell = Cell::formula(Formula::from_expr(expr)); cell.style = style; // Formulas whose result is a date/time get a format from functions like TODAY(). diff --git a/crates/engine/src/cmd/formulas.rs b/crates/engine/src/cmd/formulas.rs index 0c9eb5c3e..f3576e4f5 100644 --- a/crates/engine/src/cmd/formulas.rs +++ b/crates/engine/src/cmd/formulas.rs @@ -43,7 +43,7 @@ pub fn specs() -> Vec { "Create from Selection", ["Formulas", "Defined Names"], Some("Cmd+Shift+F3"), - "{range?, top?: true, left?: false, bottom?, right?}", + "{range?, top?: true, left?: false, bottom?: false, right?: false, replace?: false (existing names are kept and listed in `skipped`)}", has_doc, create_from_selection ), @@ -218,15 +218,26 @@ fn valid_name(n: &str) -> bool { if !(first.is_alphabetic() || first == '_' || first == '\\') { return false; } - if n.len() > 255 || n.eq_ignore_ascii_case("R") || n.eq_ignore_ascii_case("C") { - return false; - } - if CellRef::parse(n).is_some() { + if n.len() > 255 || CellRef::parse(n).is_some() || is_r1c1(n) { return false; } n.chars().all(|c| c.is_alphanumeric() || matches!(c, '_' | '.' | '\\' | '?')) } +/// Looks like an R1C1 reference (`R`, `C`, `RC`, `R2`, `C3`, `R1C1`), which Excel doesn't +/// accept as a name. +fn is_r1c1(n: &str) -> bool { + let digits = |s: &str| s.bytes().take_while(u8::is_ascii_digit).count(); + let mut rest = n; + if let Some(r) = rest.strip_prefix(['R', 'r']) { + rest = r.get(digits(r)..).unwrap_or(""); + } + if let Some(c) = rest.strip_prefix(['C', 'c']) { + rest = c.get(digits(c)..).unwrap_or(""); + } + rest.is_empty() && !n.is_empty() +} + fn define_name(s: &mut Session, p: &Json) -> Result { let Some(name) = str_param(p, "name").map(str::to_string) else { s.ui_requests.push(crate::UiRequest::Dialog("defineName".into(), json!({}))); @@ -300,6 +311,9 @@ fn create_from_selection(s: &mut Session, p: &Json) -> Result { let r = target_range(s, p)?; let top = bool_param(p, "top").unwrap_or(true); let left = bool_param(p, "left").unwrap_or(false); + let bottom = bool_param(p, "bottom").unwrap_or(false); + let right = bool_param(p, "right").unwrap_or(false); + let replace = bool_param(p, "replace").unwrap_or(false); let d = s.doc()?; let sh = d.wb.active().ok_or(EngineError::NoDocument)?; let q = gridcraft_formula::quote_sheet(&sh.name); @@ -314,28 +328,38 @@ fn create_from_selection(s: &mut Session, p: &Json) -> Result { }; let clean = |t: String| -> String { let mut n: String = t.trim().chars().map(|c| if c.is_alphanumeric() || c == '.' { c } else { '_' }).collect(); - if n.chars().next().is_some_and(|c| c.is_ascii_digit()) || CellRef::parse(&n).is_some() { + if !n.is_empty() && !valid_name(&n) { n.insert(0, '_'); } - n + if valid_name(&n) { n } else { String::new() } }; + // The labels in the chosen edge rows/columns name the cells between them. + let (r0, r1) = (r.start.row + top as u32, r.end.row.saturating_sub(bottom as u32)); + let (c0, c1) = (r.start.col + left as u32, r.end.col.saturating_sub(right as u32)); let mut made = Vec::new(); - if top && r.height() > 1 { - for c in r.start.col..=r.end.col { - let name = clean(sh.value(CellRef::new(r.start.row, c)).display()); - if !name.is_empty() { - made.push((name, abs(RangeRef::new(CellRef::new(r.start.row + 1, c), CellRef::new(r.end.row, c))))); + if r0 <= r1 && c0 <= c1 { + for (on, row) in [(top, r.start.row), (bottom, r.end.row)] { + for c in (c0..=c1).filter(|_| on) { + let name = clean(sh.value(CellRef::new(row, c)).display()); + if !name.is_empty() { + made.push((name, abs(RangeRef::new(CellRef::new(r0, c), CellRef::new(r1, c))))); + } } } - } - if left && r.width() > 1 { - for row in r.start.row + top as u32..=r.end.row { - let name = clean(sh.value(CellRef::new(row, r.start.col)).display()); - if !name.is_empty() { - made.push((name, abs(RangeRef::new(CellRef::new(row, r.start.col + 1), CellRef::new(row, r.end.col))))); + for (on, col) in [(left, r.start.col), (right, r.end.col)] { + for row in (r0..=r1).filter(|_| on) { + let name = clean(sh.value(CellRef::new(row, col)).display()); + if !name.is_empty() { + made.push((name, abs(RangeRef::new(CellRef::new(row, c0), CellRef::new(row, c1))))); + } } } } + // Excel asks before replacing an existing name; here it's kept unless `replace` is set, and + // reported. + let exists = |n: &str| d.wb.names.iter().any(|x| x.name.eq_ignore_ascii_case(n) && x.scope.is_none()); + let (made, skipped): (Vec<_>, Vec<_>) = made.into_iter().partition(|(n, _)| replace || !exists(n)); + let skipped: Vec = skipped.into_iter().map(|(n, _)| n).collect(); let n = made.len(); edit(s, |cx| { for (name, f) in made { @@ -343,7 +367,7 @@ fn create_from_selection(s: &mut Session, p: &Json) -> Result { cx.wb.names.push(DefinedName { name, scope: None, formula: f, comment: String::new(), hidden: false }); } cx.structural = true; - Ok(json!({"created": n})) + Ok(json!({"created": n, "skipped": skipped})) }) } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index e446d4f6d..ad8fa6a1e 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -283,6 +283,65 @@ fn tables_and_structured_refs() { assert_eq!(v(&s, "B4"), Value::Number(7.0)); } +#[test] +fn create_names_from_selection() { + let names = |s: &Session| -> Vec<(String, String)> { + let mut v: Vec<_> = s.doc().unwrap().wb.names.iter().map(|n| (n.name.clone(), n.formula.clone())).collect(); + v.sort(); + v + }; + let named = |n: &str, f: &str| (n.to_string(), f.to_string()); + let mut s = s(); + s.execute( + "range.setValues", + json!({"range": "A1", "values": [["", "Jan", "Feb", ""], ["North", 1, 2, "N"], ["South", 3, 4, "S"], ["", "First", "Second", ""]]}), + ) + .unwrap(); + s.execute("formulas.createFromSelection", json!({"range": "A1:D4", "top": false, "bottom": true, "right": true})).unwrap(); + assert_eq!( + names(&s), + [named("First", "Sheet1!$B$1:$B$3"), named("N", "Sheet1!$A$2:$C$2"), named("S", "Sheet1!$A$3:$C$3"), named("Second", "Sheet1!$C$1:$C$3")] + ); + // Existing names are kept unless replacing is asked for. + let r = s.execute("formulas.createFromSelection", json!({"range": "A1:C3", "top": true, "left": true})).unwrap(); + assert_eq!(r["created"], 4); + let r = s.execute("formulas.createFromSelection", json!({"range": "B1:C3", "top": true})).unwrap(); + assert_eq!((r["created"].clone(), r["skipped"].clone()), (json!(0), json!(["Jan", "Feb"]))); + s.execute("cell.set", json!({"cell": "B1", "input": "N"})).unwrap(); + let r = s.execute("formulas.createFromSelection", json!({"range": "B1:B3", "replace": true})).unwrap(); + assert_eq!(r["created"], 1); + assert!(names(&s).contains(&named("N", "Sheet1!$B$2:$B$3"))); + assert!(names(&s).contains(&named("North", "Sheet1!$B$2:$C$2"))); +} + +#[test] +fn names_that_look_like_references_are_refused() { + let mut s = s(); + for bad in ["R1C1", "r2", "C3", "RC", "R", "c", "rc12", "A1", "XFD1048576", "1st"] { + assert!(s.execute("formulas.defineName", json!({"name": bad, "refersTo": "=1"})).is_err(), "{bad}"); + } + for good in ["R1C1X", "Rate", "RCx", "Rx", "_R1", "Cost"] { + s.execute("formulas.defineName", json!({"name": good, "refersTo": "=1"})).unwrap(); + } +} + +#[test] +fn lambda_names_keep_their_case_in_calls() { + let mut s = s(); + s.execute("formulas.defineName", json!({"name": "Double", "refersTo": "=LAMBDA(x,x*2)"})).unwrap(); + s.execute("formulas.defineName", json!({"name": "Quad", "refersTo": "=LAMBDA(x,Double(Double(x)))"})).unwrap(); + s.execute("cell.set", json!({"cell": "A1", "input": "=double(4)+Quad(1)+sum(1)"})).unwrap(); + assert_eq!(s.execute("cell.get", json!({"cell": "A1"})).unwrap()["formula"], "=Double(4)+Quad(1)+SUM(1)"); + assert_eq!(v(&s, "A1"), Value::Number(13.0)); + let r = s.execute("file.saveBytes", json!({"format": "xlsx"})).unwrap(); + let b64 = r["base64"].as_str().unwrap().to_string(); + s.execute("file.open", json!({"name": "x.xlsx", "base64": b64})).unwrap(); + assert_eq!(s.execute("cell.get", json!({"cell": "A1"})).unwrap()["formula"], "=Double(4)+Quad(1)+SUM(1)"); + let quad = s.doc().unwrap().wb.names.iter().find(|n| n.name == "Quad").unwrap().formula.clone(); + assert_eq!(quad, "LAMBDA(x,Double(Double(x)))"); + assert_eq!(v(&s, "A1"), Value::Number(13.0)); +} + #[test] fn sheets() { let mut s = s(); diff --git a/crates/model/src/workbook.rs b/crates/model/src/workbook.rs index 8652b20b3..6ccef9e9c 100644 --- a/crates/model/src/workbook.rs +++ b/crates/model/src/workbook.rs @@ -3,8 +3,10 @@ use std::sync::Arc; use gridcraft_core::DateSystem; +use gridcraft_formula::Expr; use serde::{Deserialize, Serialize}; +use crate::cell::Formula; use crate::sheet::Sheet; use crate::style::{StyleTable, Theme}; @@ -143,6 +145,53 @@ impl Workbook { .find(|n| n.scope == Some(sheet) && n.name.eq_ignore_ascii_case(name)) .or_else(|| self.names.iter().find(|n| n.scope.is_none() && n.name.eq_ignore_ascii_case(name))) } + /// Spells calls of LAMBDA names the way the names are defined (`Double(4)`, where the + /// parser gives `DOUBLE(4)`: it upper-cases function names, as built-in ones are shown). + pub fn name_call_case(&self, e: Expr) -> Expr { + spell_calls(e, &self.lambda_names()) + } + /// [`Workbook::name_call_case`] for every cell formula and defined name (after reading a + /// file). + pub fn apply_name_call_case(&mut self) { + let lambdas = self.lambda_names(); + if lambdas.is_empty() { + return; + } + for sheet in self.sheets.iter_mut() { + let fixed: Vec<_> = sheet + .cells + .iter() + .filter_map(|(c, cell)| { + let f = cell.formula.as_ref()?; + let e = f.expr()?; + let ne = spell_calls(e.clone(), &lambdas); + (ne != e).then(|| (c, Formula { array: f.array, ..Formula::from_expr(ne) })) + }) + .collect(); + if fixed.is_empty() { + continue; + } + let sheet = Arc::make_mut(sheet); + for (c, f) in fixed { + if let Some(cell) = sheet.cells.get_mut(c) { + cell.formula = Some(Arc::new(f)); + } + } + } + for n in self.names.iter_mut() { + if let Ok(e) = gridcraft_formula::parse(&n.formula) { + let ne = spell_calls(e.clone(), &lambdas); + if ne != e { + n.formula = gridcraft_formula::print(&ne); + } + } + } + } + /// Names of the defined names holding a LAMBDA (callable like functions). + fn lambda_names(&self) -> Vec { + let is_lambda = |f: &str| f.trim_start_matches('=').trim_start().get(..7).is_some_and(|p| p.eq_ignore_ascii_case("LAMBDA(")); + self.names.iter().filter(|n| is_lambda(&n.formula)).map(|n| n.name.clone()).collect() + } /// Table by name across sheets: (sheet index, table index). pub fn table(&self, name: &str) -> Option<(usize, usize)> { for (si, s) in self.sheets.iter().enumerate() { @@ -172,6 +221,20 @@ impl Workbook { } } +/// Calls of the names in `lambdas` spelled as the names are. +fn spell_calls(e: Expr, lambdas: &[String]) -> Expr { + if lambdas.is_empty() { + return e; + } + e.map(&mut |x| match x { + Expr::Call(n, args) => match lambdas.iter().find(|l| l.eq_ignore_ascii_case(&n)) { + Some(l) => Expr::Call(l.clone(), args), + None => Expr::Call(n, args), + }, + other => other, + }) +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/xlsx/src/read.rs b/crates/xlsx/src/read.rs index 915d8e4f9..bf45e91fa 100644 --- a/crates/xlsx/src/read.rs +++ b/crates/xlsx/src/read.rs @@ -279,6 +279,8 @@ pub fn read_xlsx(bytes: &[u8]) -> Result<(Workbook, ReadReport), IoError> { if wb_rels.iter().any(|r| r.kind == "externalLink") { cx.warn("links to external workbooks are not supported"); } + // Calls of LAMBDA names are spelled like the names (the parser upper-cases them). + wb.apply_name_call_case(); wb.styles = cx.styles; Ok((wb, cx.report)) }