Merge pull request #755 from marcoarnulfo:fix/xludf-prefix

This commit is contained in:
Brandon Thomas 2026-10-12 23:49:23 -04:00
commit d8561a9ed7
7 changed files with 98 additions and 10 deletions

View File

@ -182,6 +182,11 @@ fn errors_and_cycles() {
assert!(t.get("B1").is_error()); assert!(t.get("B1").is_error());
t.set("C1", "=NOSUCHFN(1)"); t.set("C1", "=NOSUCHFN(1)");
assert_eq!(t.get("C1"), Value::Error(CellError::Name)); 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+"); t.set("C2", "=1+");
assert_eq!(t.get("C2"), Value::Error(CellError::Name)); assert_eq!(t.get("C2"), Value::Error(CellError::Name));
} }

View File

@ -89,15 +89,19 @@ fn word_is_cell(w: &str) -> Option<Anchor> {
Some(Anchor { row, col, row_abs, col_abs }) 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 { pub fn normalize_function_name(name: &str) -> String {
let u = name.to_ascii_uppercase(); 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) { if let Some(r) = u.strip_prefix(p) {
return r.to_string(); return r.to_string();
} }
} }
u match u.strip_prefix("_XLUDF.") {
Some(r) => format!("_xludf.{r}"),
None => u,
}
} }
impl Parser { impl Parser {

View File

@ -64,6 +64,7 @@ fn normalises() {
assert_eq!(rt("=sum(a1:b2)"), "SUM(A1:B2)"); assert_eq!(rt("=sum(a1:b2)"), "SUM(A1:B2)");
assert_eq!(rt("= 1 + 2"), "1+2"); 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("_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("true"), "TRUE");
assert_eq!(rt("'Sheet1'!A1"), "Sheet1!A1"); assert_eq!(rt("'Sheet1'!A1"), "Sheet1!A1");
} }

View File

@ -197,7 +197,8 @@ fn registry() -> &'static (Vec<FnSpec>, 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> { pub fn lookup(name: &str) -> Option<&'static FnSpec> {
let mut n = name.trim().to_ascii_uppercase(); let mut n = name.trim().to_ascii_uppercase();
loop { loop {

View File

@ -9,6 +9,14 @@ pub(crate) fn canonical(
sheet: usize, sheet: usize,
cell: gridcraft_engine::core::CellRef, cell: gridcraft_engine::core::CellRef,
) -> Result<String, ParseError> { ) -> Result<String, ParseError> {
// 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() == "@") { 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()); 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 { 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, // English (and every language without a formula table) shows the stored text, `_xludf.`
// and skips a parse per frame. // prefixes aside, and skips a parse per frame.
if locale == FormulaLocale::En { 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() }; 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())) .and_then(|expr| formula::print_input(&expr, locale, |name| wb.name(name, sheet).is_some() || wb.table(name).is_some()))
.map(|body| format!("={body}")) .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 // Descriptions/signatures for the supported Spanish function table; untranslated functions

View File

@ -1,7 +1,7 @@
//! Language selection must affect UI entry, never the canonical engine/file boundary. //! Language selection must affect UI entry, never the canonical engine/file boundary.
use gridcraft_engine::{ use gridcraft_engine::{
Session, Session,
core::{CellRef, Value}, core::{CellError, CellRef, Value},
}; };
use gridcraft_ui_egui::{SheetApp, UiState, i18n::Language}; use gridcraft_ui_egui::{SheetApp, UiState, i18n::Language};
use serde_json::json; 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(); let restored: UiState = serde_json::from_str(&serde_json::to_string(&app.ui).unwrap()).unwrap();
assert_eq!(restored.language, Language::Cs); 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\")");
}

View File

@ -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!(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)); 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#"<row r="{}"><c r="{a}"><f>{}</f></c></row>"#, i + 1, f.replace('"', "&quot;")))
.collect();
let (wb, _) = read_xlsx(&minimal(&format!("<sheetData>{rows}</sheetData>"), &[], "", "")).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}");
}
}