From 57d7ea9bc38c432a2319161c0989ea0017405ba1 Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Mon, 2 Nov 2026 02:52:38 -0500 Subject: [PATCH] Engine: a Python reference that grows or shrinks drops the cell's kept result Deleting part of a range that xl() reads (row 2 of A1:A3) shortens the reference instead of making it #REF!, and inserting rows inside the range lengthens it; either way the data the code reads changed, but the cell kept its result. rewrite_python_refs now treats a reference whose rows or columns changed in number like a deleted one: the cell is new code (#BLOCKED!) with no kept result. A reference that only moves keeps the result. Test: deleting_the_data_python_code_reads_drops_its_kept_result now covers a partial deletion and an insertion inside the range (fails without the fix). --- crates/engine/src/cmd/edit/paste_special.rs | 14 +++++++++++--- crates/engine/src/tests/python_files.rs | 15 ++++++++++++++- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/edit/paste_special.rs b/crates/engine/src/cmd/edit/paste_special.rs index b886dfc19..ff782d863 100644 --- a/crates/engine/src/cmd/edit/paste_special.rs +++ b/crates/engine/src/cmd/edit/paste_special.rs @@ -399,8 +399,9 @@ pub(crate) fn rewrite_all_formulas(wb: &mut gridcraft_model::Workbook, target: & /// cut-move, sheet rename and delete) runs it, as it adjusts formulas. A literal is only replaced /// when the reference it names changed, and text that doesn't parse is kept. A cell's cached /// result is kept while its references still name the same data (a rename, a move); a reference -/// the edit deleted (`#REF!`) leaves the result stale, so the cell is new code (`#BLOCKED!`) and -/// the recalculation removes its spill. A rewrite that would +/// the edit deleted (`#REF!`), or one that grew or shrank (rows or columns inserted or deleted +/// inside it), leaves the result stale, so the cell is new code (`#BLOCKED!`) and the +/// recalculation removes its spill. A rewrite that would /// make a cell's code longer than one cell holds (a longer sheet name, `Z1` becoming `AA1`) refuses /// the whole edit, which then changes nothing: see [`check_python_fits`]. pub(crate) fn rewrite_python_refs( @@ -418,7 +419,7 @@ pub(crate) fn rewrite_python_refs( let expr = gridcraft_formula::parse(reference).ok()?; let before = gridcraft_formula::print(&expr); let after = gridcraft_formula::print(&f(&host, expr)); - lost |= after.contains("#REF!") && !before.contains("#REF!"); + lost |= (after.contains("#REF!") && !before.contains("#REF!")) || area_shape(&before) != area_shape(&after); (after != before).then_some(after) }); if let Some(code) = changed { @@ -446,6 +447,13 @@ pub(crate) fn rewrite_python_refs( Ok(()) } +/// The rows and columns a printed reference covers (`A1:B3`, `Sheet2!$A$1`, `C:E`), or `None` +/// for any other form (a name, an error). +fn area_shape(reference: &str) -> Option<(u32, u32)> { + let r = RangeRef::parse(reference.rsplit('!').next()?)?; + Some((r.height(), r.width())) +} + /// Refuses Python code an edit has made longer than one cell holds: the limit `python.set` /// enforces and opening a file applies (code over it is dropped to its value on open). The edit /// is refused rather than made, so no code is lost and none keeps a reference the edit moved. diff --git a/crates/engine/src/tests/python_files.rs b/crates/engine/src/tests/python_files.rs index a88369d0e..cad3f8745 100644 --- a/crates/engine/src/tests/python_files.rs +++ b/crates/engine/src/tests/python_files.rs @@ -384,20 +384,33 @@ fn deleting_the_data_python_code_reads_drops_its_kept_result() { let mut s = s(); s.execute("cell.set", json!({"cell": "A1", "input": "1"})).unwrap(); s.execute("cell.set", json!({"cell": "A2", "input": "2"})).unwrap(); + s.execute("cell.set", json!({"cell": "A3", "input": "3"})).unwrap(); py(&mut s, "C5", "xl(\"A1:A2\") * 2"); cache(&mut s, "C5", array(2, 1, vec![2.0.into(), 4.0.into()]), "list"); + py(&mut s, "E5", "xl(\"A1:A3\")"); + cache(&mut s, "E5", Value::Number(3.0), "int"); s.execute("formulas.calculateNow", json!({})).unwrap(); assert_eq!(v(&s, "C6"), Value::Number(4.0)); // A move keeps the result: the reference still names the same data. s.execute("home.insertRows", json!({"rows": "1"})).unwrap(); assert!(code(&s, "C6").is_some_and(|p| p.output.is_some() && !p.code.contains("A1:A2")), "{:?}", code(&s, "C6")); assert_eq!(v(&s, "C7"), Value::Number(4.0)); - // Deleting the rows it reads leaves `#REF!`: the kept result no longer belongs to the code. + assert!(code(&s, "E6").is_some_and(|p| p.output.is_some())); + // Deleting the rows C6 reads leaves `#REF!`, and deleting part of E6's range shrinks it: in + // both, the kept result no longer belongs to the code. s.execute("home.deleteRows", json!({"rows": "2:3"})).unwrap(); let c4 = code(&s, "C4").expect("the Python cell moved up to C4"); assert!(c4.code.contains("#REF!") && c4.output.is_none(), "{c4:?}"); assert_eq!(v(&s, "C4"), Value::Error(CellError::Blocked)); assert_eq!(v(&s, "C5"), Value::Empty, "its spill is gone"); + let e4 = code(&s, "E4").expect("the Python cell moved up to E4"); + assert!(!e4.code.contains("#REF!") && e4.output.is_none(), "{e4:?}"); + assert_eq!(v(&s, "E4"), Value::Error(CellError::Blocked)); + // Rows inserted inside a range grow it: also new data. + py(&mut s, "G1", "xl(\"A1:A2\")"); + cache(&mut s, "G1", Value::Number(2.0), "int"); + s.execute("home.insertRows", json!({"rows": "2"})).unwrap(); + assert!(code(&s, "G1").is_some_and(|p| p.output.is_none()), "{:?}", code(&s, "G1")); } #[test]