From 85d32c20be4da521fd8db989d68e2b78910ac6df Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 10 Nov 2026 18:04:31 -0500 Subject: [PATCH] engine: show overlong format codes as General without hashing them per cell (#3273) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * engine: show overlong format codes as General without hashing them per cell The display cache hashed the full format code on every lookup, so one huge code shared by many cells cost its length for every cell drawn. A code with more than 4 × 255 bytes can't be within Excel's 255 characters and is now formatted as General straight away. * address review: keep the original code in the public number_format for overlong formats --- crates/engine/src/display.rs | 40 +++++++++++++++++++++++++++- crates/engine/src/io.rs | 2 +- crates/engine/src/macros/recorder.rs | 2 +- crates/engine/src/macros/run.rs | 2 +- crates/engine/src/pivot.rs | 2 +- 5 files changed, 43 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/display.rs b/crates/engine/src/display.rs index 06a0fa587..4dbdf46e8 100644 --- a/crates/engine/src/display.rs +++ b/crates/engine/src/display.rs @@ -8,8 +8,13 @@ use gridcraft_model::{Sheet, Workbook}; use gridcraft_numfmt::{Formatted, NumberFormat, format_value}; /// Parsed number formats are cached by code. -fn parsed(code: &str) -> NumberFormat { +pub(crate) fn parsed(code: &str) -> NumberFormat { static CACHE: std::sync::LazyLock>> = std::sync::LazyLock::new(|| Mutex::new(HashMap::new())); + // A code of more bytes than 4 × the character limit can't be within it: it shows as General + // without being hashed or scanned again for every cell that shares it. + if code.len() > 4 * gridcraft_numfmt::MAX_FORMAT_CODE_CHARS { + return NumberFormat::parse("General"); + } let lock = || CACHE.lock().unwrap_or_else(std::sync::PoisonError::into_inner); if let Some(f) = lock().get(code) { return f.clone(); @@ -24,7 +29,12 @@ fn parsed(code: &str) -> NumberFormat { f } +/// The parsed format of `code`; one longer than the character limit is a General format that keeps +/// the code it was made from. The engine itself formats through `parsed`, which doesn't copy it. pub fn number_format(code: &str) -> NumberFormat { + if code.len() > 4 * gridcraft_numfmt::MAX_FORMAT_CODE_CHARS { + return NumberFormat::parse(code); + } parsed(code) } @@ -228,3 +238,31 @@ pub fn stats(sheet: &Sheet, ranges: &[RangeRef]) -> Stats { } s } + +#[cfg(test)] +mod tests { + use super::*; + + /// A format code far past Excel's 255 characters shows its values as General, for every cell + /// that shares it, without being hashed into the cache each time. + #[test] + fn an_overlong_code_formats_as_general() { + let wb = Workbook::new(); + let code = format!("0.00{}", "\"x\"".repeat(400_000)); + for _ in 0..1_000 { + assert_eq!(format(&Value::Number(1.5), &code, &wb).text, "1.5"); + } + assert_eq!(format(&Value::Number(1.5), "0.00", &wb).text, "1.50"); + } + + /// The public `number_format` keeps the code of an overlong format; it is General, but not the + /// same format as a real `General`. + #[test] + fn number_format_keeps_an_overlong_code() { + let code = format!("0.00{}", "\"x\"".repeat(2_000)); + let f = number_format(&code); + assert!(f.is_general()); + assert_eq!(f.code(), code); + assert_ne!(f, number_format("General")); + } +} diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 8f2c59a97..8711785c7 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -576,7 +576,7 @@ pub fn csv_values_bytes(wb: &Workbook) -> Result> { let Some(sh) = wb.sheet(wb.active_sheet()) else { return Ok(Vec::new()) }; gridcraft_xlsx::write_csv(sh, wb.csv_delimiter.unwrap_or(b','), |c| match sh.value(c) { gridcraft_core::Value::Number(n) if n.is_finite() => { - let shape = crate::display::number_format(wb.styles.get(sh.style_id(c)).num_fmt.as_str()).date_shape(n); + let shape = crate::display::parsed(wb.styles.get(sh.style_id(c)).num_fmt.as_str()).date_shape(n); match shape.zip(gridcraft_core::date::datetime_from_serial(wb.date_system, n)) { Some(((date, time), d)) => iso(&d, date, time), None => n.to_string(), diff --git a/crates/engine/src/macros/recorder.rs b/crates/engine/src/macros/recorder.rs index 337c41661..ef9be781f 100644 --- a/crates/engine/src/macros/recorder.rs +++ b/crates/engine/src/macros/recorder.rs @@ -440,7 +440,7 @@ pub fn infer_input(wb: &Workbook, sheet: usize, r: RangeRef) -> NamedInput { Some(match v { Value::Empty => return None, Value::Text(t) if t.is_empty() => return None, - Value::Number(_) if crate::display::number_format(wb.styles.get(sh.style_id(c)).num_fmt.as_str()).is_date() => ColType::Date, + Value::Number(_) if crate::display::parsed(wb.styles.get(sh.style_id(c)).num_fmt.as_str()).is_date() => ColType::Date, Value::Number(_) => ColType::Number, Value::Text(_) => ColType::Text, Value::Bool(_) => ColType::Bool, diff --git a/crates/engine/src/macros/run.rs b/crates/engine/src/macros/run.rs index 9e31e3ca9..96c6f504a 100644 --- a/crates/engine/src/macros/run.rs +++ b/crates/engine/src/macros/run.rs @@ -335,7 +335,7 @@ fn check_input(wb: &Workbook, input: &gridcraft_model::NamedInput, place: Place) gridcraft_model::ColType::Number => v.is_number(), gridcraft_model::ColType::Text => v.is_text(), gridcraft_model::ColType::Bool => matches!(v, Value::Bool(_)), - gridcraft_model::ColType::Date => v.is_number() && crate::display::number_format(&date()).is_date(), + gridcraft_model::ColType::Date => v.is_number() && crate::display::parsed(&date()).is_date(), }; if !ok { wrong.push((c, holds(&v, ty))); diff --git a/crates/engine/src/pivot.rs b/crates/engine/src/pivot.rs index bfaccace4..1b5d48155 100644 --- a/crates/engine/src/pivot.rs +++ b/crates/engine/src/pivot.rs @@ -182,7 +182,7 @@ pub fn read_source(wb: &Workbook, text: &str, default_sheet: usize) -> Result