From f63a437d5b9d0d26c718c96779637762ea71c181 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Mon, 12 Oct 2026 19:07:32 +0200 Subject: [PATCH 1/2] Formulas: keep _xludf. so user-defined calls stay user-defined (fixes #726) The parser stripped _xludf. like _xlfn., but the two mean opposite things: _xludf. marks a name Excel must not treat as a built-in. Keeping it in the call name makes the call an unknown function (#NAME?, as in Excel) and the writer emits it unchanged, so _xludf.EOMONTH() no longer saves as an EOMONTH() call Excel repairs away. --- crates/calc/src/tests.rs | 5 +++++ crates/formula/src/parser.rs | 10 +++++++--- crates/formula/src/tests.rs | 1 + crates/functions/src/lib.rs | 3 ++- crates/xlsx/src/tests/fixtures.rs | 25 +++++++++++++++++++++++++ 5 files changed, 40 insertions(+), 4 deletions(-) diff --git a/crates/calc/src/tests.rs b/crates/calc/src/tests.rs index 5133409..6214206 100644 --- a/crates/calc/src/tests.rs +++ b/crates/calc/src/tests.rs @@ -167,6 +167,11 @@ fn errors_and_cycles() { assert!(t.get("B1").is_error()); t.set("C1", "=NOSUCHFN(1)"); assert_eq!(t.get("C1"), Value::Error(CellError::Name)); + // `_xludf.` marks a user-defined function, never the built-in of the same name (#726). + for f in ["=_xludf.CONCAT(\"a\",\"b\")", "=_xludf.SUM(1,2)", "=_xludf.EOMONTH()"] { + t.set("C3", f); + assert_eq!(t.get("C3"), Value::Error(CellError::Name), "{f}"); + } t.set("C2", "=1+"); assert_eq!(t.get("C2"), Value::Error(CellError::Name)); } diff --git a/crates/formula/src/parser.rs b/crates/formula/src/parser.rs index 4fd3b7b..81b8fe0 100644 --- a/crates/formula/src/parser.rs +++ b/crates/formula/src/parser.rs @@ -88,15 +88,19 @@ fn word_is_cell(w: &str) -> Option { Some(Anchor { row, col, row_abs, col_abs }) } -/// `_xlfn.XLOOKUP` → `XLOOKUP`. +/// `_xlfn.XLOOKUP` → `XLOOKUP`. `_xludf.` is kept (`_xludf.SUM`): it marks a user-defined +/// function, never the built-in of the same name, and files need it back. pub fn normalize_function_name(name: &str) -> String { let u = name.to_ascii_uppercase(); - for p in ["_XLFN._XLWS.", "_XLFN.", "_XLWS.", "_XLUDF."] { + for p in ["_XLFN._XLWS.", "_XLFN.", "_XLWS."] { if let Some(r) = u.strip_prefix(p) { return r.to_string(); } } - u + match u.strip_prefix("_XLUDF.") { + Some(r) => format!("_xludf.{r}"), + None => u, + } } impl Parser { diff --git a/crates/formula/src/tests.rs b/crates/formula/src/tests.rs index d3dd4c2..69491f6 100644 --- a/crates/formula/src/tests.rs +++ b/crates/formula/src/tests.rs @@ -48,6 +48,7 @@ fn normalises() { assert_eq!(rt("=sum(a1:b2)"), "SUM(A1:B2)"); assert_eq!(rt("= 1 + 2"), "1+2"); assert_eq!(rt("_xlfn.XLOOKUP(1,A:A,B:B)"), "XLOOKUP(1,A:A,B:B)"); + assert_eq!(rt("_XLUDF.concat(\"a\",\"b\")"), "_xludf.CONCAT(\"a\",\"b\")"); assert_eq!(rt("true"), "TRUE"); assert_eq!(rt("'Sheet1'!A1"), "Sheet1!A1"); } diff --git a/crates/functions/src/lib.rs b/crates/functions/src/lib.rs index 282b230..65612b4 100644 --- a/crates/functions/src/lib.rs +++ b/crates/functions/src/lib.rs @@ -197,7 +197,8 @@ fn registry() -> &'static (Vec, HashMap<&'static str, usize>) { }) } -/// Looks a function up by name (case-insensitive; `_xlfn.`, `_xlws.` and `_xludf.`-free names). +/// Looks a function up by name (case-insensitive; `_xlfn.`- and `_xlws.`-free names). A +/// `_xludf.` name is user-defined and never found. pub fn lookup(name: &str) -> Option<&'static FnSpec> { let mut n = name.trim().to_ascii_uppercase(); loop { diff --git a/crates/xlsx/src/tests/fixtures.rs b/crates/xlsx/src/tests/fixtures.rs index 47dde5a..99221dd 100644 --- a/crates/xlsx/src/tests/fixtures.rs +++ b/crates/xlsx/src/tests/fixtures.rs @@ -495,3 +495,28 @@ fn text_box_formatting_is_read_and_written() { assert_eq!(rgb(g.text_outline.map(|o| o.0)), rgb(f.text_outline.map(|o| o.0))); assert_eq!(g.text_outline.map(|o| o.1), Some(1.5)); } + +/// `_xludf.` marks a user-defined call even when a built-in has the same name; dropping it turns +/// the call into the built-in, and `_xludf.EOMONTH()` into a call Excel repairs away (#726). +#[test] +fn user_defined_function_prefix_round_trips() { + let cases = + [("A1", "_xludf.EOMONTH()"), ("A2", "_xludf.CONCAT(\"a\",\"b\")"), ("A3", "_xludf.SUM(1,2)"), ("A4", "MYUDF(1)"), ("A5", "_xludf.MYUDF(1)")]; + let rows: String = cases + .iter() + .enumerate() + .map(|(i, (a, f))| format!(r#"{}"#, i + 1, f.replace('"', """))) + .collect(); + let (wb, _) = read_xlsx(&minimal(&format!("{rows}"), &[], "", "")).unwrap(); + let s = wb.sheet(0).unwrap(); + for (a, f) in cases { + assert_eq!(s.cell(at(a)).unwrap().formula.as_ref().unwrap().text, f, "read {a}"); + } + let bytes = crate::write_xlsx(&wb).unwrap(); + let mut pkg = crate::package::Package::open(&bytes).unwrap(); + let sheet = pkg.read_xml("xl/worksheets/sheet1.xml").unwrap().unwrap(); + for (a, f) in cases { + let cell = sheet.child("sheetData").unwrap().kids("row").flat_map(|r| r.kids("c")).find(|c| c.attr("r") == Some(a)).unwrap(); + assert_eq!(cell.child("f").unwrap().text, f, "written {a}"); + } +} From 6939d95caee24b5298cff9a6cede4adabf7b43e1 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Mon, 12 Oct 2026 22:53:10 +0200 Subject: [PATCH 2/2] Formula bar: hide _xludf. like Excel, keep it when the edit is confirmed unchanged (#726) --- crates/ui-egui/src/formula_locale.rs | 44 +++++++++++++++++++++++--- crates/ui-egui/tests/formula_locale.rs | 20 +++++++++++- 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/crates/ui-egui/src/formula_locale.rs b/crates/ui-egui/src/formula_locale.rs index ad0725e..6e833b9 100644 --- a/crates/ui-egui/src/formula_locale.rs +++ b/crates/ui-egui/src/formula_locale.rs @@ -9,6 +9,14 @@ pub(crate) fn canonical( sheet: usize, cell: gridcraft_engine::core::CellRef, ) -> Result { + // The editor hid the `_xludf.` prefixes of this cell's formula; confirming it unchanged keeps + // the user-defined calls instead of binding them to the built-ins of the same name. + if let Some(stored) = wb.sheet(sheet).and_then(|sh| sh.cell(cell)).filter(|c| c.formula.is_some()).map(|c| c.input_text()) + && has_udf_prefix(&stored) + && text == display(&stored, locale, wb, sheet) + { + return Ok(stored); + } if locale == FormulaLocale::En || wb.sheet(sheet).is_some_and(|sh| wb.styles.get(sh.style_id(cell)).num_fmt.as_str() == "@") { return Ok(text.to_string()); } @@ -40,16 +48,42 @@ pub(crate) fn canonical( } pub(crate) fn display(text: &str, locale: FormulaLocale, wb: &Workbook, sheet: usize) -> String { - // English (and every language without a formula table) shows the stored text untouched, - // and skips a parse per frame. + // English (and every language without a formula table) shows the stored text, `_xludf.` + // prefixes aside, and skips a parse per frame. if locale == FormulaLocale::En { - return text.to_string(); + return hide_udf_prefix(text, locale); } let Some(body) = text.strip_prefix('=') else { return text.to_string() }; - formula::parse(body) + let shown = formula::parse(body) .and_then(|expr| formula::print_input(&expr, locale, |name| wb.name(name, sheet).is_some() || wb.table(name).is_some())) .map(|body| format!("={body}")) - .unwrap_or_else(|_| text.to_string()) + .unwrap_or_else(|_| text.to_string()); + hide_udf_prefix(&shown, locale) +} + +const UDF_PREFIX: &str = "_xludf."; + +fn has_udf_prefix(text: &str) -> bool { + text.to_ascii_lowercase().contains(UDF_PREFIX) +} + +/// Excel shows a user-defined call such as `_xludf.EOMONTH()` as `=EOMONTH()`. Only function +/// names lose the prefix: text literals and names keep it. +fn hide_udf_prefix(text: &str, locale: FormulaLocale) -> String { + let Some(body) = text.strip_prefix('=').filter(|b| has_udf_prefix(b)) else { return text.to_string() }; + let Ok(tokens) = formula::lexer::tokenize_locale(body, locale) else { return text.to_string() }; + let mut out = String::from("="); + let mut from = 0; + for t in tokens { + if matches!(t.tok, formula::lexer::Tok::Func(_)) + && body.get(t.start..t.start + UDF_PREFIX.len()).is_some_and(|p| p.eq_ignore_ascii_case(UDF_PREFIX)) + { + out.push_str(body.get(from..t.start).unwrap_or("")); + from = t.start + UDF_PREFIX.len(); + } + } + out.push_str(body.get(from..).unwrap_or("")); + out } // Descriptions/signatures for the supported Spanish function table; untranslated functions diff --git a/crates/ui-egui/tests/formula_locale.rs b/crates/ui-egui/tests/formula_locale.rs index fe21218..8558693 100644 --- a/crates/ui-egui/tests/formula_locale.rs +++ b/crates/ui-egui/tests/formula_locale.rs @@ -1,7 +1,7 @@ //! Language selection must affect UI entry, never the canonical engine/file boundary. use gridcraft_engine::{ Session, - core::{CellRef, Value}, + core::{CellError, CellRef, Value}, }; use gridcraft_ui_egui::{SheetApp, UiState, i18n::Language}; use serde_json::json; @@ -201,3 +201,21 @@ fn czech_interface_retains_its_canonical_formula_fallback() { let restored: UiState = serde_json::from_str(&serde_json::to_string(&app.ui).unwrap()).unwrap(); assert_eq!(restored.language, Language::Cs); } + +#[test] +fn user_defined_prefix_is_hidden_while_editing_and_kept_on_commit() { + // Excel shows `_xludf.EOMONTH()` as `=EOMONTH()`; the cell still holds the user-defined call + // (#726), and committing the edit unchanged must not turn it into the built-in. + let mut app = app(); + app.run("cell.set", json!({"cell":"A1","input":"=_xludf.EOMONTH()&_xludf.CONCAT(\"_xludf.x\")"})).unwrap(); + app.begin_edit(None, true); + assert_eq!(app.editor.as_ref().unwrap().text, "=EOMONTH()&CONCAT(\"_xludf.x\")"); + assert!(app.commit_edit(0, 0, false, false)); + assert_eq!(formula(&app), "_xludf.EOMONTH()&_xludf.CONCAT(\"_xludf.x\")"); + assert_eq!(app.session.active().unwrap().wb.active().unwrap().value(CellRef::new(0, 0)), Value::Error(CellError::Name)); + app.run("app.language.set", json!({"language":"es"})).unwrap(); + app.begin_edit(None, true); + assert_eq!(app.editor.as_ref().unwrap().text, "=EOMONTH()&CONCAT(\"_xludf.x\")"); + assert!(app.commit_edit(0, 0, false, false)); + assert_eq!(formula(&app), "_xludf.EOMONTH()&_xludf.CONCAT(\"_xludf.x\")"); +}