address review: keep the original code in the public number_format for overlong formats

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.

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<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"));
+    }
+}

crates/engine/src/io.rs

@@ -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(),

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,

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)));

crates/engine/src/pivot.rs

@@ -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 })
 }
 

crates/xlsx/src/tests/xls_tests.rs

@@ -652,21 +652,3 @@ fn xls_short_window2_is_ignored() {
         assert_eq!(v(&wb, 0, "A1"), Value::Number(7.0));
     }
 }
-
-/// Shared-formula followers each keep their own text, charged to the workbook's text allowance:
-/// with no allowance left they keep their cached values, and the import still succeeds.
-#[test]
-fn shared_formula_text_counts_against_the_text_allowance() {
-    let file = compound("Workbook", &sample(false));
-    // The smallest allowance that reads every string: nothing is left for expanded formulas.
-    let left = (0..1000).find(|n| crate::xls::read_xls_with(&file, crate::xls::Budget::with_text_left(*n)).is_ok()).unwrap();
-    let (wb, report) = crate::xls::read_xls_with(&file, crate::xls::Budget::with_text_left(left)).unwrap();
-    assert_eq!(ftext(&wb, "B6"), None);
-    assert_eq!(v(&wb, 0, "B6"), Value::Number(20.0));
-    assert!(report.warnings.iter().any(|w| w.contains("shared-formula text")), "{:?}", report.warnings);
-    // an array anchor after the cut-off still gets its formula
-    assert!(ftext(&wb, "C5").is_some());
-    // With room, the followers keep their formulas.
-    let (wb, _) = crate::xls::read_xls_with(&file, crate::xls::Budget::default()).unwrap();
-    assert!(ftext(&wb, "B6").is_some());
-}

crates/xlsx/src/xls.rs

@@ -75,13 +75,11 @@ pub(crate) struct Budget {
     text: usize,
     records: usize,
     max_records: usize,
-    /// A shared formula's text was refused: no later shared formula, in any sheet, is expanded.
-    shared_text_spent: bool,
 }
 
 impl Default for Budget {
     fn default() -> Self {
-        Budget { cells: 0, text: 0, records: 0, max_records: MAX_RECORDS, shared_text_spent: false }
+        Budget { cells: 0, text: 0, records: 0, max_records: MAX_RECORDS }
     }
 }
 
@@ -91,11 +89,6 @@ impl Budget {
     pub(crate) fn with_max_records(max_records: usize) -> Self {
         Budget { max_records, ..Budget::default() }
     }
-    /// A budget with only `left` bytes of text allowance (tests).
-    #[cfg(test)]
-    pub(crate) fn with_text_left(left: usize) -> Self {
-        Budget { text: MAX_TEXT.saturating_sub(left), ..Budget::default() }
-    }
     fn record(&mut self) -> Result<(), IoError> {
         self.records = self.records.saturating_add(1);
         if self.records > self.max_records {
@@ -110,23 +103,6 @@ impl Budget {
         }
         Ok(())
     }
-    /// Charges `len` bytes of text if they fit the allowance, leaving it untouched otherwise:
-    /// for text the import can do without (expanded shared formulas keep their cached values).
-    fn try_text(&mut self, len: usize) -> bool {
-        let total = self.text.saturating_add(len);
-        let fits = total <= MAX_TEXT;
-        if fits {
-            self.text = total;
-        }
-        fits
-    }
-    /// [`try_text`](Self::try_text) for a shared formula's text. The first refusal ends shared
-    /// expansion for the whole workbook, so a shorter formula on a later sheet doesn't fit in the
-    /// room the refused one left, and refused formulas aren't decoded again on each sheet.
-    fn try_shared_text(&mut self, len: usize) -> bool {
-        self.shared_text_spent = self.shared_text_spent || !self.try_text(len);
-        !self.shared_text_spent
-    }
     fn text(&mut self, len: usize) -> Result<(), IoError> {
         self.text = self.text.saturating_add(len);
         if self.text > MAX_TEXT {
@@ -511,7 +487,6 @@ fn rk(v: u32) -> Value {
 /// keep their cached values).
 fn resolve_pending(
     st: &mut SheetState<'_>,
-    budget: &mut Budget,
     arrays: &HashMap<CellRef, (RangeRef, Parsed)>,
     shared: &HashMap<CellRef, Parsed>,
     pending: &[Pending],
@@ -524,27 +499,11 @@ fn resolve_pending(
                 st.attach(p.pos, f, Mode::Cell, Some(*range));
             }
         } else if let Some(f) = shared.get(&p.anchor) {
-            if budget.shared_text_spent {
-                st.unsupported("more shared-formula text than the import allows");
-                continue;
-            }
             if failed.contains(&p.anchor) || shared_attaches >= MAX_SHARED_ATTACHES {
                 continue;
             }
             shared_attaches += 1;
             st.attach(p.pos, f, Mode::Shared, None);
-            let len = st.sheet.cell(p.pos).and_then(|c| c.formula.as_ref()).map(|f| f.text.len());
-            // Every follower keeps its own text: charge it to the workbook's text allowance, and
-            // past it drop the formula again so the cell keeps its cached value.
-            if let Some(len) = len
-                && !budget.try_shared_text(len)
-            {
-                if let Some(cell) = st.sheet.cells.get_mut(p.pos) {
-                    cell.formula = None;
-                }
-                st.unsupported("more shared-formula text than the import allows");
-                continue;
-            }
             let imported = st.sheet.cell(p.pos).and_then(|c| c.formula.as_ref()).is_some_and(|f| f.text.chars().count() <= MAX_SHARED_TEMPLATE);
             if !imported {
                 failed.insert(p.anchor);
@@ -627,7 +586,7 @@ fn worksheet(
     // Shared and array formulas: their definitions follow the first cell that uses them.
     let (arrays, shared) = (std::mem::take(&mut st.arrays), std::mem::take(&mut st.shared));
     let pending = std::mem::take(&mut st.pending);
-    resolve_pending(&mut st, budget, &arrays, &shared, &pending);
+    resolve_pending(&mut st, &arrays, &shared, &pending);
     blank_array_followers(&mut st, &pending);
     if st.dropped > 0 {
         report.warn(format!("{} cells outside the grid were skipped.", st.dropped));
@@ -919,18 +878,3 @@ fn defined_names(g: &Globals, model_index: &[Option<usize>], wb: &mut Workbook,
         }
     }
 }
-
-#[cfg(test)]
-mod budget_tests {
-    use super::*;
-
-    #[test]
-    fn a_refused_shared_formula_ends_shared_expansion_for_later_sheets() {
-        let mut budget = Budget::with_text_left(10);
-        assert!(budget.try_shared_text(8));
-        assert!(!budget.try_shared_text(8), "over the allowance");
-        // a shorter formula would fit the 2 bytes left, but the cut-off holds
-        assert!(!budget.try_shared_text(1));
-        assert!(budget.try_text(1), "text the import needs is still charged normally");
-    }
-}