mirror of
https://github.com/storytold/gridcraft.git
synced 2026-11-11 12:53:59 +00:00
engine: show overlong format codes as General without hashing them per cell (#3273)
* 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
This commit is contained in:
1 parent
1deb108a7c
commit
85d32c20be
5 files changed
+43
-5
No files matched your search
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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"));
|
||||
}
|
||||
}
|
||||
@@ -576,7 +576,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(),
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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)));
|
||||
|
||||
@@ -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 })
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user