Numfmt: TEXT() and chart labels parse each format code once (#806)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
vangel76 2026-10-12 22:09:09 +02:00
parent fc45b7f6d8
commit fd53de7c46
5 changed files with 249 additions and 3 deletions

View File

@ -132,6 +132,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in
| Date | What landed |
|---|---|
| 2026-10-12 | `TEXT()` over a range and chart labels parse each number format code once instead of once per value (#806) |
| 2026-10-12 | File › Print selects a system printer, copies, duplex and grayscale, submits PDF jobs through craft-print on a worker and shows spooler errors; browser printing and PDF export remain available |
| 2026-10-11 | Command-sequence fuzzing: random command sequences checked against undo/redo, XLSX round-trip and full recalculation; fixes for the table, legacy-array, spill, AutoSum, Watch Window and sheet-copy bugs it found |
| 2026-10-11 | Password-protected workbooks open: GridCraft asks for the password and decrypts Agile and Standard encryption (#11) |

View File

@ -122,7 +122,7 @@ impl Fmt {
pub fn new(code: &str) -> Fmt {
let t = code.trim();
let general = t.is_empty() || t.eq_ignore_ascii_case("general");
Fmt { general, fmt: NumberFormat::parse(if general { "General" } else { t }) }
Fmt { general, fmt: NumberFormat::parse_cached(if general { "General" } else { t }) }
}
/// Formats `v`; `step` (axis labels) fixes the decimals for General.
pub fn format(&self, v: f64, step: Option<f64>) -> String {

View File

@ -145,7 +145,7 @@ fn category_texts(wb: &Workbook, sheet: usize, formula: &str) -> Vec<String> {
if let Some(c) = at {
let code = &wb.styles.get(s.style_id(c)).num_fmt.0;
if !code.eq_ignore_ascii_case("general") {
return format_value(v, &NumberFormat::parse(code), wb.date_system).text;
return format_value(v, &NumberFormat::parse_cached(code), wb.date_system).text;
}
}
}

View File

@ -101,6 +101,8 @@ impl Default for NumberFormat {
impl NumberFormat {
/// Parses a format code. Never fails: unknown or malformed parts degrade to literal text.
pub fn parse(code: &str) -> NumberFormat {
#[cfg(test)]
parse_cache::count_parse();
let trimmed = code.trim();
// `checkbox` marks a checkbox cell (Insert › Checkbox): TRUE/FALSE draw as a box, any other
// value displays as General.
@ -294,6 +296,78 @@ pub fn format_value(v: &Value, fmt: &NumberFormat, sys: DateSystem) -> Formatted
}
}
/// A small per-thread cache of parsed format codes for callers that format many values with the
/// same code (`TEXT` over a range, chart labels). A parsed format depends on the code text only
/// (the date system and values are applied when formatting), so the code is the whole key.
mod parse_cache {
use std::cell::RefCell;
use std::collections::HashMap;
use super::NumberFormat;
/// Most codes kept per thread. A full cache is emptied, so memory stays bounded.
pub(super) const CAPACITY: usize = 64;
/// Longer codes are parsed every time and never stored.
pub(super) const MAX_CODE_LEN: usize = 256;
thread_local! {
static CACHE: RefCell<HashMap<String, NumberFormat>> = RefCell::new(HashMap::new());
}
pub(super) fn get(code: &str) -> NumberFormat {
if code.len() > MAX_CODE_LEN {
return NumberFormat::parse(code);
}
// Not usable (thread shutting down, re-entered): parse without the cache.
let cached = CACHE
.try_with(|c| {
let mut c = c.try_borrow_mut().ok()?;
if let Some(f) = c.get(code) {
return Some(f.clone());
}
let f = NumberFormat::parse(code);
if c.len() >= CAPACITY {
c.clear();
}
c.insert(code.to_string(), f.clone());
Some(f)
})
.ok()
.flatten();
cached.unwrap_or_else(|| NumberFormat::parse(code))
}
#[cfg(test)]
thread_local! {
static PARSES: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
}
#[cfg(test)]
pub(super) fn count_parse() {
let _ = PARSES.try_with(|p| p.set(p.get() + 1));
}
/// Parses done on this thread so far (tests only).
#[cfg(test)]
pub(super) fn parses() -> usize {
PARSES.try_with(std::cell::Cell::get).unwrap_or(0)
}
/// Empties this thread's cache (tests only).
#[cfg(test)]
pub(super) fn clear() {
let _ = CACHE.try_with(|c| c.borrow_mut().clear());
}
}
impl NumberFormat {
/// Like [`NumberFormat::parse`], but remembers recently parsed codes (per thread, bounded) so
/// formatting many values with one code parses it once. Same result as `parse`.
pub fn parse_cached(code: &str) -> NumberFormat {
parse_cache::get(code)
}
}
/// The `TEXT()` worksheet function: formats a value with a format code. Numeric text is
/// converted to a number first; other text and booleans (as `TRUE`/`FALSE`) go through the text
/// section, or are returned as is.
@ -301,7 +375,7 @@ pub fn text_function(v: &Value, code: &str, sys: DateSystem) -> Result<String, C
if code.is_empty() {
return Ok(String::new());
}
let fmt = NumberFormat::parse(code);
let fmt = NumberFormat::parse_cached(code);
let n = match v.scalar() {
Value::Error(e) => return Err(e),
Value::Empty => 0.0,

View File

@ -582,3 +582,174 @@ fn decimal_digits_keeps_fifteen_digits_and_truncates_a_tie() {
let (d, e) = decimal_digits(999999999999999.6);
assert_eq!((d, e), ([1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], 15));
}
// ---- parse cache ----
/// `TEXT()` as it was before the cache: parse the code, then format.
fn text_uncached(v: &Value, code: &str, sys: DateSystem) -> Result<String, CellError> {
if code.is_empty() {
return Ok(String::new());
}
let fmt = NumberFormat::parse(code);
let n = match v.scalar() {
Value::Error(e) => return Err(e),
Value::Empty => 0.0,
Value::Number(n) => n,
Value::Bool(b) => return Ok(fmt.format_text(if b { "TRUE" } else { "FALSE" }).text),
Value::Text(t) => match gridcraft_core::parse::parse_number_text(&t) {
Some(n) => n,
None => return Ok(fmt.format_text(&t).text),
},
Value::Array(_) => return Err(CellError::Value),
};
if !n.is_finite() {
return Err(CellError::Num);
}
let (f, overflow) = fmt.format_number(n, sys);
if overflow { Err(CellError::Value) } else { Ok(f.text) }
}
const CACHE_CODES: &[&str] = &[
"0",
"0.00",
"#,##0.00",
"#,##0.00;[Red](#,##0.00)",
"$#,##0.00",
"0%",
"0.0%",
"0.00E+00",
"##0.0E+0",
"General",
"general",
"checkbox",
"@",
"0;@",
"0;-0;;@",
"00000",
"# ?/?",
"# ??/16",
"?/?",
"m/d/yyyy",
"yyyy-mm-dd hh:mm:ss",
"dddd, mmmm d, yyyy",
"h:mm AM/PM",
"[h]:mm:ss",
"mm:ss.000",
"[Red]0;[Blue]-0",
"[>100]0.0;[<5]0.000;0",
"[<=9999999]###-####;(###) ###-####",
"[$-409]d mmm yy",
"\"abc\"0",
"\\x0",
"*-0",
"_(* #,##0_)",
"0.0.0",
"[",
"\"abc",
"0;0;0;0;0",
"#,##0.00;[Red",
"[$-]0",
" 0.00 ",
"[Color99]0",
"0.00_);(0.00)",
"#,,\"M\"",
"yyyy",
"",
];
fn cache_values() -> Vec<Value> {
vec![
Value::Number(0.0),
Value::Number(1234.5678),
Value::Number(-42.5),
Value::Number(0.5),
Value::Number(45000.75),
Value::Number(1e15),
Value::Number(1e-7),
Value::Number(-0.0001),
Value::Number(f64::INFINITY),
Value::from("abc"),
Value::from("12.5"),
Value::Bool(true),
Value::Empty,
Value::Error(CellError::NA),
]
}
#[test]
fn parse_cached_gives_the_same_results_cold_and_warm() {
assert!(CACHE_CODES.len() >= 40);
parse_cache::clear();
let values = cache_values();
for sys in [DateSystem::D1900, DateSystem::D1904] {
// Cold on the first system, warm on the second.
for code in CACHE_CODES {
let direct = NumberFormat::parse(code);
let cached = NumberFormat::parse_cached(code);
assert_eq!(cached.code(), direct.code(), "{code:?}");
assert_eq!(
(cached.kind(), cached.is_date(), cached.is_text(), cached.is_general()),
(direct.kind(), direct.is_date(), direct.is_text(), direct.is_general())
);
for v in &values {
let want = text_uncached(v, code, sys);
assert_eq!(text_function(v, code, sys), want, "TEXT({v:?}, {code:?})");
// Again, now certainly warm.
assert_eq!(text_function(v, code, sys), want, "warm TEXT({v:?}, {code:?})");
assert_eq!(format_value(v, &cached, sys), format_value(v, &direct, sys), "{v:?} {code:?}");
}
}
}
}
#[test]
fn parse_cached_parses_each_code_once() {
parse_cache::clear();
let before = parse_cache::parses();
for i in 0..10_000 {
assert!(text_function(&Value::Number(f64::from(i) + 0.5), "0.00", S).is_ok());
}
assert_eq!(parse_cache::parses() - before, 1);
let before = parse_cache::parses();
for i in 0..10_000 {
let code = if i % 2 == 0 { "0.00" } else { "#,##0.0" };
assert!(text_function(&Value::Number(f64::from(i)), code, S).is_ok());
}
// "0.00" is still cached from above: only the second code is new.
assert_eq!(parse_cache::parses() - before, 1);
parse_cache::clear();
let before = parse_cache::parses();
for i in 0..10_000 {
let code = if i % 2 == 0 { "0.00" } else { "#,##0.0" };
assert!(text_function(&Value::Number(f64::from(i)), code, S).is_ok());
}
assert_eq!(parse_cache::parses() - before, 2);
}
#[test]
fn parse_cache_is_bounded() {
parse_cache::clear();
// More distinct codes than the capacity: all still format correctly...
let n = parse_cache::CAPACITY + 10;
for i in 0..n {
let c = format!("0.00\"{i}\"");
assert_eq!(text_function(&Value::Number(1.0), &c, S), Ok(format!("1.00{i}")));
}
// ...the first ones were evicted (parsed again), the newest is still cached.
let before = parse_cache::parses();
let newest = format!("0.00\"{}\"", n - 1);
assert!(text_function(&Value::Number(1.0), &newest, S).is_ok());
assert_eq!(parse_cache::parses() - before, 0);
assert!(text_function(&Value::Number(1.0), "0.00\"0\"", S).is_ok());
assert_eq!(parse_cache::parses() - before, 1);
// A very long code is parsed on every call and not stored.
let long = format!("0\"{}\"", "x".repeat(parse_cache::MAX_CODE_LEN));
let before = parse_cache::parses();
for _ in 0..3 {
assert!(text_function(&Value::Number(1.0), &long, S).is_ok());
}
assert_eq!(parse_cache::parses() - before, 3);
}