Use existing craft-xml escaping for XLSX output

Cargo.lock

@@ -2349,6 +2349,7 @@ dependencies = [
  "craft-offcrypto",
  "craft-opc",
  "craft-xls",
+ "craft-xml",
  "gridcraft-core",
  "gridcraft-formula",
  "gridcraft-functions",

Cargo.toml

@@ -40,6 +40,7 @@ craft-opc = { git = "https://github.com/storytold/craft-libs", rev = "bb57d422d0
 # turn them into workbooks.
 craft-csv = { git = "https://github.com/storytold/craft-libs", rev = "bb57d422d0bc842cbde430955825553ab7b976f4", version = "0.1.0" }
 # Excel 97-2003 (.xls, BIFF8) workbooks; `crates/xlsx/src/xls.rs` maps them onto the model.
+craft-xml = { git = "https://github.com/storytold/craft-libs", rev = "bb57d422d0bc842cbde430955825553ab7b976f4", version = "0.1.0" }
 craft-xls = { git = "https://github.com/storytold/craft-libs", rev = "bb57d422d0bc842cbde430955825553ab7b976f4", version = "0.1.0" }
 
 # foundation

crates/calc/src/recalc.rs

@@ -48,8 +48,6 @@ pub struct Calc {
     /// Cells whose formulas have errors from cycles.
     pub circular: Vec<Key>,
     pub last_recalc_cells: usize,
-    /// Which anchors held a spill when the current calculation started (see [`Standing`]).
-    standing: Standing,
     /// The cells whose value is `#SPILL!`, kept so that finding the formulas an emptied cell or a
     /// freed spill area may unblock doesn't walk every formula or window of cells. Filled in
     /// `rebuild`, kept up to date by `cells_changed` and by the write-back of a run; an entry is
@@ -57,45 +55,6 @@ pub struct Calc {
     spill_errors: SpillErrors,
 }
 
-/// Who gets the cells when two arrays want the same ones. Excel (measured by a reviewer on
-/// PR #2483) lets the spill that already exists keep its cells, whichever anchor comes first;
-/// the other formula shows `#SPILL!`, and clearing the winner lets it spill. The rule here:
-///
-/// 1. an anchor that held a spill before this calculation outranks one that did not;
-/// 2. otherwise the anchor first in reading order (row, then column) wins.
-///
-/// Rule 2 alone decides when nothing is remembered, so the result never depends on hash order
-/// or evaluation order. A full recalculation remembers the layout the sheet held before it
-/// started (rule 1), so it agrees with the incremental one; a workbook that is calculated for
-/// the first time has no such layout (rule 2 only).
-///
-/// Known limits: (a) the xlsx reader does not restore spill layouts, so a workbook opened from a
-/// file is laid out by rule 2 alone, although Excel keeps the layout the file records; (b) when
-/// both anchors held a spill and one grows into the other, rule 2 decides, where Excel keeps
-/// the cells the other already owns.
-#[derive(Clone, Debug, Default)]
-struct Standing {
-    /// `true`: `keys` are the anchors that held a spill (full recalculation, which lays every
-    /// spill out afresh). `false`: `keys` are the formulas that held none (incremental).
-    held: bool,
-    keys: HashSet<Key>,
-}
-
-impl Standing {
-    fn had_spill(&self, k: Key) -> bool {
-        self.keys.contains(&k) == self.held
-    }
-
-    /// Whether `a` takes cells from `b` when both want them.
-    fn outranks(&self, a: Key, b: Key) -> bool {
-        match (self.had_spill(a), self.had_spill(b)) {
-            (true, false) => true,
-            (false, true) => false,
-            _ => (a.1.row, a.1.col) < (b.1.row, b.1.col),
-        }
-    }
-}
-
 /// A set of cells by row, each row's columns in their own set: a window of rows and columns is
 /// found by one column range per row that has an entry, so the cost follows the cells in the
 /// window, not those to the side of it in the same rows.
@@ -142,7 +101,6 @@ impl Default for Calc {
             fixed_now: None,
             circular: vec![],
             last_recalc_cells: 0,
-            standing: Standing::default(),
             spill_errors: SpillErrors::default(),
         }
     }
@@ -311,14 +269,6 @@ impl Calc {
     /// Recalculates every formula (F9 / Ctrl+Alt+F9, and after loading).
     pub fn recalc_all(&mut self, wb: &mut Workbook) {
         self.rebuild(wb);
-        // The layout the sheets hold now decides who keeps cells two arrays both want.
-        // (A Python cell that holds a spill counts too, as it does in the incremental pass.)
-        let mut held = HashSet::default();
-        for i in 0..wb.sheets.len() {
-            if let Some(sh) = wb.sheet(i) {
-                held.extend(sh.spill_ranges.keys().filter(|a| sh.cell(**a).is_some_and(|c| c.formula.is_some() || c.py.is_some())).map(|a| (i, *a)));
-            }
-        }
         // Spills are laid out again from the formulas where they are now (a structural edit moves
         // or deletes the formulas, not the values they spilled).
         for i in 0..wb.sheets.len() {
@@ -330,9 +280,7 @@ impl Calc {
             }
         }
         let all: Vec<Key> = self.graph.nodes.keys().copied().collect();
-        self.standing = Standing { held: true, keys: held };
         self.run(wb, all, true);
-        self.standing = Standing::default();
     }
 
     /// Updates the graph for cells whose content changed and recalculates what depends on them.
@@ -443,13 +391,6 @@ impl Calc {
                 }
             }
         }
-        // So can the area a removed formula spilled over.
-        for (si, r) in &cleared {
-            match emptied.iter_mut().find(|x| x.0 == *si) {
-                Some(x) => x.1 = CellRef::new(x.1.row.max(r.end.row), x.1.col.max(r.end.col)),
-                None => emptied.push((*si, r.end)),
-            }
-        }
         // An emptied cell can unblock an anchor any distance above or to the left of it.
         for (si, end) in emptied {
             seeds.extend(self.unblock_candidates(wb, si, end));
@@ -531,22 +472,10 @@ impl Calc {
     fn run(&mut self, wb: &mut Workbook, mut dirty: Vec<Key>, full: bool) {
         let mut dynamic_again = !self.graph.dynamic.is_empty();
         self.last_recalc_cells = 0;
-        let mut seen: HashSet<Key> = HashSet::default();
-        if !full {
-            self.standing = Standing::default();
-        }
         for _round in 0..8 {
             if dirty.is_empty() {
                 break;
             }
-            if !full {
-                // What each formula held before this calculation touched it.
-                for &k in &dirty {
-                    if seen.insert(k) && wb.sheet(k.0).is_some_and(|s| !s.spill_ranges.contains_key(&k.1)) {
-                        self.standing.keys.insert(k);
-                    }
-                }
-            }
             // Row-major order is a good topological guess; on-demand evaluation fixes the rest.
             dirty.sort_by_key(|(s, c)| (*s, c.row, c.col));
             let mut exprs: HashMap<Key, std::sync::Arc<Expr>> = HashMap::with_capacity_and_hasher(dirty.len(), Default::default());
@@ -564,7 +493,6 @@ impl Calc {
             let tp = prof_now();
             let (results, spills, cycles) = {
                 let mut host = PassHost::new(wb, &exprs, &mut rng, now).with_python(python);
-                host.standing = Some(&self.standing);
                 host.iterate = wb.calc.iterative;
                 for k in &dirty {
                     host.calculate(*k);
@@ -663,40 +591,7 @@ impl Calc {
             if let Some(array) = new {
                 let end = CellRef::new(key.1.row.saturating_add(array.rows as u32 - 1), key.1.col.saturating_add(array.cols as u32 - 1));
                 let range = RangeRef::new(key.1, end);
-                // Two arrays that want the same cells (see [`Standing`]): a spill already laid out
-                // that this anchor outranks is taken over, and recalculated.
-                let standing = &self.standing;
-                // A spill can only be in the way where it has cells besides its anchor, and an
-                // anchor itself blocks the array. When no spilled cell lies in the range, the owners
-                // are not scanned: a full recalculation of many disjoint spills stays linear.
-                let outranked: Vec<(CellRef, RangeRef)> = if range.iter().any(|cell| cell != key.1 && sheet.spill.contains_key(&cell)) {
-                    #[cfg(test)]
-                    OWNER_SCANS.with(|n| n.set(n.get() + 1));
-                    sheet
-                        .spill_ranges
-                        .iter()
-                        .filter(|(o, r)| **o != key.1 && r.intersects(&range) && standing.outranks(*key, (key.0, **o)))
-                        .map(|(o, r)| (*o, *r))
-                        .collect()
-                } else {
-                    Vec::new()
-                };
-                let blocked = spill_is_blocked(sheet, *key, range, end, standing);
-                if !blocked {
-                    for (o, r) in outranked {
-                        for c in r.iter() {
-                            sheet.spill.remove(&c);
-                        }
-                        sheet.spill_ranges.remove(&o);
-                        if let Some(cell) = sheet.cells.get_mut(o) {
-                            cell.value = Value::Error(CellError::Spill);
-                            self.spill_errors.insert((key.0, o));
-                        }
-                        spill_changes.push((key.0, o));
-                        self.graph.dependents_of_range(key.0, r, &mut spill_changes);
-                        freed.push((key.0, r));
-                    }
-                }
+                let blocked = spill_is_blocked(sheet, key.1, range, end);
                 if blocked {
                     if let Some(cell) = sheet.cells.get_mut(key.1) {
                         cell.value = Value::Error(CellError::Spill);
@@ -733,22 +628,12 @@ impl Calc {
     }
 }
 
-// Owner scans done by `write_back`, to check that disjoint spills never walk every owner.
-#[cfg(test)]
-thread_local! {
-    pub(crate) static OWNER_SCANS: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
-}
-
-/// Whether the array at `anchor` can't be laid out over `range`. A spill already laid out that
-/// `anchor` outranks does not block it: it is taken over when written back.
-fn spill_is_blocked(sheet: &gridcraft_model::Sheet, anchor: Key, range: RangeRef, end: CellRef, standing: &Standing) -> bool {
-    let outranked =
-        |cell: CellRef| sheet.spill_ranges.iter().any(|(o, r)| *o != anchor.1 && r.contains(cell) && standing.outranks(anchor, (anchor.0, *o)));
+fn spill_is_blocked(sheet: &gridcraft_model::Sheet, anchor: CellRef, range: RangeRef, end: CellRef) -> bool {
     end.row >= gridcraft_core::MAX_ROWS
         || end.col >= gridcraft_core::MAX_COLS
-        || range.iter().any(|cell| cell != anchor.1 && (sheet.cells.has(cell) || (sheet.spill.contains_key(&cell) && !outranked(cell))))
+        || range.iter().any(|cell| cell != anchor && (sheet.cells.has(cell) || sheet.spill.contains_key(&cell)))
         || sheet.merges.iter().any(|merge| merge.intersects(&range))
-        || sheet.tables.iter().any(|table| table.range.contains(anchor.1))
+        || sheet.tables.iter().any(|table| table.range.contains(anchor))
 }
 
 fn fill_spill(sheet: &mut gridcraft_model::Sheet, anchor: CellRef, range: RangeRef, array: &Array) {

crates/calc/src/recalc/pass_host.rs

@@ -31,8 +31,6 @@ pub(super) struct PassHost<'a> {
     /// every row, so once per sheet rather than once per range read.
     used: HashMap<usize, Option<RangeRef>>,
     rng: &'a mut u64,
-    /// Who held a spill before the calculation; unset: every formula did (reading order decides).
-    pub(super) standing: Option<&'a super::Standing>,
     now: f64,
     cycle: bool,
     pub(super) cycles: Vec<Key>,
@@ -101,7 +99,6 @@ impl<'a> PassHost<'a> {
             spill_index: SpillIndex::default(),
             used: HashMap::default(),
             rng,
-            standing: None,
             now,
             cycle: false,
             cycles: vec![],
@@ -163,26 +160,19 @@ fn spill_hits_overlap(index: &SpillIndex, sheet: usize, hits: &[CellRef], range:
 }
 
 /// Whether an array result at `k` can't spill: it is in a table, or its area runs off the sheet,
-/// overlaps merged cells, holds other cells, or the spill of another formula that outranks it
-/// (see `Standing`; unless that formula is recalculated in this pass, and so may spill elsewhere
-/// now). A spill it outranks gives way to it when written back.
+/// overlaps merged cells, holds other cells, or another formula's spill (unless that formula is
+/// recalculated in this pass, and so may spill elsewhere now).
 fn spill_blocked(host: &PassHost<'_>, (sheet, anchor): Key, a: &Array) -> bool {
     let Some(sh) = host.wb.sheet(sheet) else { return false };
     let end =
         CellRef::new(anchor.row.saturating_add((a.rows as u32).saturating_sub(1)), anchor.col.saturating_add((a.cols as u32).saturating_sub(1)));
     let range = RangeRef::new(anchor, end);
     let own = sh.spill_ranges.get(&anchor);
-    let default = super::Standing::default();
-    let standing = host.standing.unwrap_or(&default);
     let spilled_over = |c: CellRef| {
         !own.is_some_and(|o| o.contains(c))
             && sh.spill_ranges.iter().any(|(o, r)| {
                 // A formula or Python cell recalculated in this pass lays its array out again.
-                *o != anchor
-                    && r.contains(c)
-                    && !host.exprs.contains_key(&(sheet, *o))
-                    && !host.python.contains_key(&(sheet, *o))
-                    && standing.outranks((sheet, *o), (sheet, anchor))
+                *o != anchor && r.contains(c) && !host.exprs.contains_key(&(sheet, *o)) && !host.python.contains_key(&(sheet, *o))
             })
     };
     end.row >= gridcraft_core::MAX_ROWS

crates/calc/src/tests.rs

@@ -2960,105 +2960,6 @@ fn formulas_read_a_dirty_python_cell_after_it_is_laid_out() {
     assert_eq!((t.num("A1"), t.num("A2"), t.num("A3")), (5.0, 100.0, 111.0));
 }
 
-#[test]
-fn disjoint_spills_do_not_scan_every_spill_owner() {
-    // Many independent two-cell arrays: write-back must not walk all earlier owners for each one.
-    let mut t = T::new();
-    for i in 1..=2000 {
-        t.wb.sheet_mut(0).unwrap().cells.set(c(&format!("A{}", i * 2 - 1)), Cell::formula(Formula::new("=SEQUENCE(2)")));
-    }
-    crate::recalc::OWNER_SCANS.with(|n| n.set(0));
-    t.calc.recalc_all(&mut t.wb);
-    assert_eq!((t.num("A2"), t.num("A4000")), (2.0, 2.0));
-    let scans = crate::recalc::OWNER_SCANS.with(|n| n.get());
-    assert!(scans < 10, "{scans} owner scans for 2000 disjoint spills");
-}
-
-#[test]
-fn overlapping_spill_edit_sequences_match_a_full_recalc() {
-    // Random short edit sequences on a small grid of arrays that run into each other (sizes from
-    // shared cells, so one edit resizes several at once), with constants and clears in their way.
-    // After every edit the incremental result must equal a full recalc of the same workbook.
-    let seeds: u64 = std::env::var("GRIDCRAFT_SPILL_SEEDS").ok().and_then(|v| v.parse().ok()).unwrap_or(200);
-    let steps: usize = std::env::var("GRIDCRAFT_SPILL_STEPS").ok().and_then(|v| v.parse().ok()).unwrap_or(20);
-    let only: Option<u64> = std::env::var("GRIDCRAFT_SPILL_SEED").ok().and_then(|v| v.parse().ok());
-    let next = |state: &mut u64| {
-        *state = state.wrapping_add(0x9E37_79B9_7F4A_7C15);
-        let mut z = *state;
-        z = (z ^ (z >> 30)).wrapping_mul(0xBF58_476D_1CE4_E5B9);
-        z = (z ^ (z >> 27)).wrapping_mul(0x94D0_49BB_1331_11EB);
-        z ^ (z >> 31)
-    };
-    let seeds: Vec<u64> = match only {
-        Some(s) => vec![s],
-        None => (0..seeds).collect(),
-    };
-    for seed in seeds {
-        let mut state = seed;
-        let mut t = T::new();
-        // Sizes in J1:J4.
-        for (i, n) in [2, 3, 1, 4].iter().enumerate() {
-            t.set(&format!("J{}", i + 1), &n.to_string());
-        }
-        let mut log: Vec<(String, String)> = Vec::new();
-        for step in 0..steps {
-            let r = next(&mut state);
-            let at = format!("{}{}", (b'A' + (r % 6) as u8) as char, 1 + (r >> 8) % 6);
-            let kind = (r >> 16) % 14;
-            let (at, input) = match kind {
-                0..=3 => {
-                    let a = 1 + (r >> 24) % 4;
-                    let b = 1 + (r >> 28) % 4;
-                    (at, format!("=SEQUENCE($J${a},$J${b})"))
-                }
-                4 => (at, format!("=SEQUENCE({},{})", 1 + (r >> 24) % 3, 1 + (r >> 28) % 3)),
-                5 => (format!("J{}", 1 + (r >> 24) % 4), (1 + (r >> 28) % 4).to_string()),
-                6 => (at, "x".to_string()),
-                7 | 8 => (at, String::new()),
-                9 => (format!("K{}", 1 + (r >> 24) % 6), "=SUM(A1:F6)".to_string()),
-                10 => {
-                    // An earlier edit again (often putting back what a cell held before, as undo does).
-                    match log.len() {
-                        0 => (at, String::new()),
-                        n => log[(r >> 24) as usize % n].clone(),
-                    }
-                }
-                11 => (format!("{}{}", (b'A' + ((r >> 24) % 6) as u8) as char, 1 + (r >> 32) % 6), (1 + (r >> 40) % 3).to_string()),
-                // Several cells in one edit (a fill, or clearing a range): a 2x2 block from `at`.
-                12 => (format!("{at}+"), String::new()),
-                _ => (format!("{at}+"), format!("=SEQUENCE($J${},$J${})", 1 + (r >> 24) % 4, 1 + (r >> 28) % 4)),
-            };
-            log.push((at.clone(), input.clone()));
-            if let Some(from) = at.strip_suffix('+') {
-                let from = c(from);
-                let keys: Vec<(usize, CellRef)> =
-                    (0..2).flat_map(|dr| (0..2).map(move |dc| (0, CellRef { row: from.row + dr, col: from.col + dc }))).collect();
-                for k in &keys {
-                    let cell = if input.is_empty() { Cell::value(Value::Empty) } else { Cell::formula(Formula::new(&input)) };
-                    t.wb.sheet_mut(0).unwrap().cells.set(k.1, cell);
-                }
-                t.calc.cells_changed(&mut t.wb, &keys);
-            } else {
-                t.set(&at, &input);
-            }
-            let mut fresh = t.wb.clone();
-            Calc::new().recalc_all(&mut fresh);
-            let (a, b) = (t.wb.sheet(0).unwrap(), fresh.sheet(0).unwrap());
-            for row in 0..12 {
-                for col in 0..12 {
-                    let cell = CellRef { row, col };
-                    assert_eq!(a.value(cell), b.value(cell), "seed {seed} step {step}: {cell} incremental vs full recalc after {log:?}");
-                }
-            }
-            let mut ka: Vec<_> = a.spill_ranges.iter().map(|(k, v)| (*k, *v)).collect();
-            let mut kb: Vec<_> = b.spill_ranges.iter().map(|(k, v)| (*k, *v)).collect();
-            ka.sort_by_key(|(k, _)| (k.row, k.col));
-            kb.sort_by_key(|(k, _)| (k.row, k.col));
-            assert_eq!(ka, kb, "seed {seed} step {step}: spill layout after {log:?}");
-        }
-    }
-}
-
 /// A computed criterion whose formula runs a database function over the same criteria counts as
 /// not met inside itself: before, each record's check ran the function again for every record, at
 /// every level, so the work grew exponentially with the nesting allowed.

crates/engine/src/tests.rs

@@ -2940,127 +2940,6 @@ fn validation_formulas_are_relative_and_see_the_new_value() {
     assert!(!ok(&mut s, "C1", "50"));
 }
 
-/// Every value of the active sheet as the incremental recalculation left it must equal a full one.
-fn assert_matches_full_recalc(s: &Session) {
-    let wb = &s.doc().unwrap().wb;
-    let mut fresh = (**wb).clone();
-    gridcraft_calc::Calc::new().recalc_all(&mut fresh);
-    let (a, b) = (wb.active().unwrap(), fresh.active().unwrap());
-    for row in 0..30 {
-        for col in 0..12 {
-            let cell = CellRef { row, col };
-            assert_eq!(a.value(cell), b.value(cell), "{cell} in memory vs after a full recalc");
-        }
-    }
-}
-
-#[test]
-fn filling_a_formula_next_to_a_spill_matches_a_full_recalc() {
-    // Found by the command-sequence fuzzer: C14 spills along row 14; filling `=D14:E18` over
-    // E13:F13 puts two array formulas whose areas run into that spill and into each other. The
-    // incremental pass left F13 as #SPILL! while a full recalc gave F13 its array and C14
-    // #SPILL!. The spill that already exists keeps its cells (as in Excel), in both.
-    let mut s = s();
-    s.execute("range.setValues", json!({"range": "B18", "values": [[1], [2], [3], [4], [5], [6], [7]]})).unwrap();
-    s.execute("cell.set", json!({"cell": "C14", "input": "=TRANSPOSE(B18:B24)"})).unwrap();
-    s.execute("selection.set", json!({"range": "E13"})).unwrap();
-    s.execute("range.fill", json!({"range": "E13:F13", "input": "=D14:E18"})).unwrap();
-    assert_eq!(v(&s, "C14"), Value::Number(1.0));
-    assert_eq!(v(&s, "I14"), Value::Number(7.0));
-    assert_eq!(v(&s, "F13"), Value::Error(gridcraft_core::CellError::Spill));
-    assert_matches_full_recalc(&s);
-}
-
-#[test]
-fn overlapping_spills_keep_the_one_that_exists_whichever_anchor_is_first() {
-    use gridcraft_core::CellError::Spill;
-    // E10 spills along row 10, F8 down over F10: F8 is first in reading order, E10 may be first
-    // to spill. Excel keeps whichever spilled first.
-    for (first, second) in [(("E10", "=SEQUENCE(1,3)"), ("F8", "=SEQUENCE(5)")), (("F8", "=SEQUENCE(5)"), ("E10", "=SEQUENCE(1,3)"))] {
-        let mut s = s();
-        s.execute("cell.set", json!({"cell": first.0, "input": first.1})).unwrap();
-        s.execute("cell.set", json!({"cell": second.0, "input": second.1})).unwrap();
-        assert_ne!(v(&s, first.0), Value::Error(Spill), "{} keeps its cells", first.0);
-        assert_eq!(v(&s, second.0), Value::Error(Spill), "{} is blocked", second.0);
-        assert_matches_full_recalc(&s);
-        // Clearing the winner lets the other one spill.
-        s.execute("edit.clearContents", json!({"range": first.0})).unwrap();
-        assert_ne!(v(&s, second.0), Value::Error(Spill), "{} spills once {} is cleared", second.0, first.0);
-        assert_matches_full_recalc(&s);
-    }
-}
-
-#[test]
-fn a_spill_growing_into_another_and_back_matches_a_full_recalc_at_every_step() {
-    use gridcraft_core::CellError::Spill;
-    // Both arrays already spill; F8 (first in reading order) grows down over E10's spill, takes
-    // its cells, shrinks back and lets it spill again, then the steps are undone, redone, edited
-    // inside the contested area and cleared. Every state must match a full recalc.
-    let mut s = s();
-    s.execute("cell.set", json!({"cell": "H1", "input": "1"})).unwrap();
-    s.execute("cell.set", json!({"cell": "F8", "input": "=SEQUENCE(H1)"})).unwrap();
-    s.execute("cell.set", json!({"cell": "E10", "input": "=SEQUENCE(1,3)*10"})).unwrap();
-    assert_eq!(v(&s, "G10"), Value::Number(30.0));
-    assert_matches_full_recalc(&s);
-    let steps: [(&str, serde_json::Value); 9] = [
-        ("cell.set", json!({"cell": "H1", "input": "5"})),
-        ("cell.set", json!({"cell": "E10", "input": "=SEQUENCE(1,2)*10"})),
-        ("cell.set", json!({"cell": "H1", "input": "1"})),
-        ("edit.undo", json!({})),
-        ("edit.undo", json!({})),
-        ("edit.redo", json!({})),
-        ("cell.set", json!({"cell": "F11", "input": "x"})),
-        ("cell.set", json!({"cell": "H1", "input": "2"})),
-        ("edit.clearContents", json!({"range": "E8:F10"})),
-    ];
-    for (i, (id, params)) in steps.iter().enumerate() {
-        s.execute(id, params.clone()).unwrap();
-        assert_matches_full_recalc(&s);
-        if i == 0 {
-            // Both held a spill: reading order gives F8 the cells.
-            assert_eq!(v(&s, "F12"), Value::Number(5.0));
-            assert_eq!(v(&s, "E10"), Value::Error(Spill));
-        }
-        if i == 2 {
-            assert_eq!(v(&s, "E10"), Value::Number(10.0));
-            assert_eq!(v(&s, "F10"), Value::Number(20.0));
-        }
-        if i == 3 {
-            // Undone back to the grown F8.
-            assert_eq!(v(&s, "F12"), Value::Number(5.0));
-            assert_eq!(v(&s, "E10"), Value::Error(Spill));
-        }
-        if i == 8 {
-            assert_eq!((v(&s, "F8"), v(&s, "E10"), v(&s, "F9")), (Value::Empty, Value::Empty, Value::Empty));
-        }
-    }
-}
-
-#[test]
-fn overlapping_spills_with_no_history_go_to_the_earlier_anchor() {
-    use gridcraft_core::CellError::Spill;
-    // A workbook calculated for the first time (a file that carries no layout) has no entry
-    // order: the anchor first in reading order wins, whatever order the formulas were set in.
-    for order in [["E10", "F8"], ["F8", "E10"]] {
-        let mut s = s();
-        for a in order {
-            let input = if a == "E10" { "=SEQUENCE(1,3)" } else { "=SEQUENCE(5)" };
-            s.execute("cell.set", json!({"cell": a, "input": input})).unwrap();
-        }
-        let mut fresh = (*s.doc().unwrap().wb).clone();
-        for i in 0..fresh.sheets.len() {
-            let sh = fresh.sheet_mut(i).unwrap();
-            sh.spill.clear();
-            sh.spill_ranges.clear();
-        }
-        gridcraft_calc::Calc::new().recalc_all(&mut fresh);
-        let a = fresh.active().unwrap();
-        let cell = |r: &str| a.value(CellRef::parse(r).unwrap());
-        assert_ne!(cell("F8"), Value::Error(Spill), "order {order:?}");
-        assert_eq!(cell("E10"), Value::Error(Spill), "order {order:?}");
-    }
-}
-
 #[test]
 fn multi_area_rule_formulas_are_relative_to_the_top_left_cell_of_all_areas() {
     // Issue #3152: Excel writes the formula for the top-left cell of all the areas (H26), whatever

crates/xlsx/Cargo.toml

@@ -23,6 +23,7 @@ craft-csv = { workspace = true }
 craft-offcrypto = { workspace = true }
 craft-opc = { workspace = true }
 craft-xls = { workspace = true }
+craft-xml = { workspace = true }
 thiserror = { workspace = true }
 quick-xml = { workspace = true }
 zip = { workspace = true }

crates/xlsx/src/xml.rs

@@ -359,31 +359,9 @@ pub fn parse_streaming(xml: &[u8], split: &[&str], cb: &mut dyn FnMut(El) -> Res
 
 // ---------------------------------------------------------------- writing helpers
 
-/// Escapes text content.
-pub fn esc(s: &str) -> String {
-    let mut o = String::with_capacity(s.len());
-    push_esc(&mut o, s);
-    o
-}
-
-/// Appends [`esc`]`(s)` to `o`.
-pub fn push_esc(o: &mut String, s: &str) {
-    if s.bytes().all(|b| matches!(b, b'\t' | b'\n' | 0x20..=0x7E) && !matches!(b, b'&' | b'<' | b'>')) {
-        o.push_str(s);
-        return;
-    }
-    o.reserve(s.len());
-    for c in s.chars() {
-        match c {
-            '&' => o.push_str("&amp;"),
-            '<' => o.push_str("&lt;"),
-            '>' => o.push_str("&gt;"),
-            '\r' => o.push_str("&#13;"),
-            c if is_xml_char(c) => o.push(c),
-            _ => {}
-        }
-    }
-}
+// XML 1.0 byte escaping is shared; spreadsheet string and phonetic policy stays here.
+use craft_xml::is_xml_char;
+pub use craft_xml::{esc, esc_attr, push_esc};
 
 /// Appends `n` in decimal.
 pub fn push_u64(o: &mut String, mut n: u64) {
@@ -422,29 +400,6 @@ pub fn push_a1(o: &mut String, c: gridcraft_core::CellRef) {
     push_u64(o, u64::from(c.row) + 1);
 }
 
-/// Escapes an attribute value (double-quoted).
-pub fn esc_attr(s: &str) -> String {
-    let mut o = String::with_capacity(s.len());
-    for c in s.chars() {
-        match c {
-            '&' => o.push_str("&amp;"),
-            '<' => o.push_str("&lt;"),
-            '>' => o.push_str("&gt;"),
-            '"' => o.push_str("&quot;"),
-            '\n' => o.push_str("&#10;"),
-            '\r' => o.push_str("&#13;"),
-            '\t' => o.push_str("&#9;"),
-            c if is_xml_char(c) => o.push(c),
-            _ => {}
-        }
-    }
-    o
-}
-
-fn is_xml_char(c: char) -> bool {
-    matches!(c, '\t' | '\n' | '\r' | '\u{20}'..='\u{D7FF}' | '\u{E000}'..='\u{FFFD}' | '\u{10000}'..='\u{10FFFF}')
-}
-
 /// ST_Xstring encoding for cell text: characters XML can't carry and carriage returns become
 /// `_xHHHH_`, and literal `_xHHHH_` sequences are protected with `_x005F_`. The result still needs
 /// [`esc`].
@@ -805,3 +760,19 @@ mod tests {
         assert_eq!("1.5E20".parse::<f64>().unwrap(), 1.5e20);
     }
 }
+
+#[cfg(test)]
+mod shared_escape_parity {
+    use super::*;
+    #[test]
+    fn xml_content_and_attributes_preserve_grid_bytes() {
+        let value = "&<>\"'\t\n\r\0\u{1f}\u{fffe}\u{ffff}é😀";
+        assert_eq!(esc(value), "&amp;&lt;&gt;\"'\t\n&#13;é😀");
+        assert_eq!(esc_attr(value), "&amp;&lt;&gt;&quot;'&#9;&#10;&#13;é😀");
+        let mut appended = "prefix:".to_owned();
+        push_esc(&mut appended, value);
+        assert_eq!(appended, format!("prefix:{}", esc(value)));
+        assert_eq!(esc("plain ASCII\t\n"), "plain ASCII\t\n");
+        assert_eq!(decode_xstring(&encode_xstring("_x000D_\r\0😀")), "_x000D_\r\0😀");
+    }
+}