Compare commits

...
Author SHA1 Message Date
Dylan M. Taylor 734b8c7755 address review: keep the original code in the public number_format for overlong formats 2026-11-04 01:41:46 +00:00
Dylan M. Taylor 6cdd6db150 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.
2026-11-03 14:07:22 +00:00
5 changed files with 43 additions and 5 deletions
+39 -1
View File
@@ -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<Mutex<HashMap<String, NumberFormat>>> = 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)
}
@@ -226,3 +236,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"));
}
}
+1 -1
View File
@@ -443,7 +443,7 @@ pub fn csv_values_bytes(wb: &Workbook) -> Result<Vec<u8>> {
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(),
+1 -1
View File
@@ -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,
+1 -1
View File
@@ -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)));
+1 -1
View File
@@ -182,7 +182,7 @@ pub fn read_source(wb: &Workbook, text: &str, default_sheet: usize) -> Result<So
for (n, a) in numeric.iter_mut().zip(any.iter()) {
*n = *n && *a;
}
let dates = numeric.iter().zip(formats.iter()).map(|(n, f)| *n && crate::display::number_format(f).is_date()).collect();
let dates = numeric.iter().zip(formats.iter()).map(|(n, f)| *n && crate::display::parsed(f).is_date()).collect();
Ok(Source { sheet: si, range: r, headers, formats, numeric, dates, rows })
}