diff --git a/crates/engine/src/cmd/edit/paste_special.rs b/crates/engine/src/cmd/edit/paste_special.rs index df2b0de61..c90dd9ec8 100644 --- a/crates/engine/src/cmd/edit/paste_special.rs +++ b/crates/engine/src/cmd/edit/paste_special.rs @@ -567,19 +567,19 @@ pub(crate) fn rewrite_all_formulas(wb: &mut gridcraft_model::Workbook, target: & let on_target = moves_arrays && host.eq_ignore_ascii_case(target); let Some(sheet) = wb.sheet_mut(si) else { continue }; for (c, f) in keys { - let Some(expr) = f.expr() else { continue }; - let new = gridcraft_formula::adjust::adjust(expr, &host, target, e); - let text = gridcraft_formula::print(&new); + // Borrows the tree and copies and prints only the formulas the edit changes. A formula + // the edit leaves alone keeps its stored text, even if that text is not canonical. + let Some(new) = f.with_expr(|x| gridcraft_formula::adjust::adjust_if_changed(x, &host, target, e)) else { continue }; let array = match f.array { Some(a) if on_target => crate::cmd::sheet::map_range(a, e), a => a, }; - if (text != f.text || array != f.array) - && let Some(cell) = sheet.cells.get_mut(c) - { - let mut nf = Formula::from_expr(new); - nf.array = array; - nf.dynamic = f.dynamic; + let nf = match new { + Some(new) => Formula { array, dynamic: f.dynamic, ..Formula::from_expr(new) }, + None if array != f.array => Formula { array, ..(*f).clone() }, + None => continue, + }; + if let Some(cell) = sheet.cells.get_mut(c) { cell.formula = Some(std::sync::Arc::new(nf)); } } diff --git a/crates/engine/src/cmd/sheet/tabs.rs b/crates/engine/src/cmd/sheet/tabs.rs index d21851553..2c1657ef9 100644 --- a/crates/engine/src/cmd/sheet/tabs.rs +++ b/crates/engine/src/cmd/sheet/tabs.rs @@ -55,11 +55,12 @@ pub(super) fn delete_sheet(s: &mut Session, p: &Json) -> Result { cx.wb.sheets.get(si).map(|s| s.cells.iter().filter_map(|(c, x)| x.formula.clone().map(|f| (c, f))).collect()).unwrap_or_default(); let Some(sh) = cx.wb.sheet_mut(si) else { continue }; for (c, f) in keys { - if let Some(e) = f.expr() { - let ne = gridcraft_formula::adjust::delete_sheet(e, &name, &order); - if let Some(cell) = sh.cells.get_mut(c) { - cell.formula = Some(Arc::new(gridcraft_model::Formula::from_expr(ne))); - } + // Only formulas whose tree changes are rewritten; the others keep their text. + let changed = f.with_expr(|e| Some(gridcraft_formula::adjust::delete_sheet(e.clone(), &name, &order)).filter(|ne| ne != e)); + if let Some(Some(ne)) = changed + && let Some(cell) = sh.cells.get_mut(c) + { + cell.formula = Some(Arc::new(gridcraft_model::Formula::from_expr(ne))); } } } @@ -109,14 +110,12 @@ pub(super) fn rename_sheet(s: &mut Session, p: &Json) -> Result { cx.wb.sheets.get(si).map(|s| s.cells.iter().filter_map(|(c, x)| x.formula.clone().map(|f| (c, f))).collect()).unwrap_or_default(); let Some(sh) = cx.wb.sheet_mut(si) else { continue }; for (c, f) in keys { - if let Some(e) = f.expr() { - let ne = gridcraft_formula::adjust::rename_sheet(e, &old, &name); - let text = gridcraft_formula::print(&ne); - if text != f.text - && let Some(cell) = sh.cells.get_mut(c) - { - cell.formula = Some(Arc::new(gridcraft_model::Formula::from_expr(ne))); - } + // Compares trees, so unchanged formulas are not printed; they keep their text. + let changed = f.with_expr(|e| Some(gridcraft_formula::adjust::rename_sheet(e.clone(), &old, &name)).filter(|ne| ne != e)); + if let Some(Some(ne)) = changed + && let Some(cell) = sh.cells.get_mut(c) + { + cell.formula = Some(Arc::new(gridcraft_model::Formula::from_expr(ne))); } } for ch in sh.charts.iter_mut() { diff --git a/crates/engine/tests/it/main.rs b/crates/engine/tests/it/main.rs index 0ba7c0db8..ca960d759 100644 --- a/crates/engine/tests/it/main.rs +++ b/crates/engine/tests/it/main.rs @@ -29,6 +29,7 @@ mod sheet_copy_spill; mod sheet_name_quoting; mod sparse_clear_invalidation; mod stored_list_shape; +mod structural_edit_rewrite; mod table_resize_overlap; mod table_total_row_last_row; mod trace_error; diff --git a/crates/engine/tests/it/structural_edit_rewrite.rs b/crates/engine/tests/it/structural_edit_rewrite.rs new file mode 100644 index 000000000..bb87ba3c4 --- /dev/null +++ b/crates/engine/tests/it/structural_edit_rewrite.rs @@ -0,0 +1,84 @@ +//! Structural edits (insert and delete of lines, sheet rename and delete) rewrite only the formulas +//! whose tree they change. A formula they leave alone keeps its stored text and its flags. + +use std::sync::Arc; + +use gridcraft_core::{CellRef, RangeRef}; +use gridcraft_engine::Session; +use gridcraft_model::{Cell, Formula}; +use serde_json::{Value as Json, json}; + +fn run(s: &mut Session, command: &str, params: Json) { + s.execute(command, params).unwrap(); +} + +fn workbook() -> Session { + let mut s = Session::new(); + s.new_workbook(); + run(&mut s, "home.insertSheet", json!({"name": "Other"})); + run(&mut s, "home.insertSheet", json!({"name": "Spare"})); + run(&mut s, "sheet.activate", json!({"sheet": "Sheet1"})); + s +} + +fn formula(s: &Session, sheet: &str, cell: &str) -> Arc { + let wb = &s.doc().unwrap().wb; + wb.sheet(wb.sheet_index(sheet).unwrap()).unwrap().cell(CellRef::parse(cell).unwrap()).unwrap().formula.clone().unwrap() +} + +/// Stores a formula with this exact text and no cached tree, as a JSON workbook loads it. +fn put_raw(s: &mut Session, cell: &str, text: &str) { + let wb = Arc::make_mut(&mut s.doc_mut().unwrap().wb); + let i = wb.sheet_index("Sheet1").unwrap(); + let f = Formula { text: text.to_string(), expr: None, array: None, dynamic: false }; + wb.sheet_mut(i).unwrap().cells.set(CellRef::parse(cell).unwrap(), Cell::formula(f)); +} + +#[test] +fn an_edit_that_leaves_a_formula_alone_keeps_its_stored_text() { + // The rewrite compares trees, not printed text, so a stored non-canonical text is not + // re-printed by an edit that does not change the formula. An edit that changes it prints it. + let mut s = workbook(); + put_raw(&mut s, "C1", "sum(a1, Other!b2)"); + run(&mut s, "home.insertRows", json!({"rows": "5:5"})); + run(&mut s, "home.insertColumns", json!({"cols": "F:F"})); + run(&mut s, "sheet.rename", json!({"sheet": "Spare", "name": "Spare2"})); + run(&mut s, "home.deleteSheet", json!({"sheet": "Spare2"})); + assert_eq!(formula(&s, "Sheet1", "C1").text, "sum(a1, Other!b2)"); + + run(&mut s, "home.insertRows", json!({"rows": "1:1"})); + assert_eq!(formula(&s, "Sheet1", "C2").text, "SUM(A2,Other!B2)"); +} + +#[test] +fn renaming_a_referenced_sheet_rewrites_the_formula() { + let mut s = workbook(); + put_raw(&mut s, "C1", "sum(a1, Other!b2)"); + run(&mut s, "sheet.rename", json!({"sheet": "Other", "name": "Data"})); + assert_eq!(formula(&s, "Sheet1", "C1").text, "SUM(A1,Data!B2)"); + run(&mut s, "home.deleteSheet", json!({"sheet": "Data"})); + assert_eq!(formula(&s, "Sheet1", "C1").text, "SUM(A1,#REF!)"); +} + +#[test] +fn inserting_rows_above_a_legacy_array_moves_its_range_when_the_formula_is_unchanged() { + let mut s = workbook(); + run(&mut s, "range.setValues", json!({"range": "B1", "values": [[1], [2]]})); + run(&mut s, "selection.set", json!({"range": "C5:C6"})); + run(&mut s, "cell.set", json!({"cell": "C5", "input": "=B1:B2*2", "array": true})); + assert_eq!(formula(&s, "Sheet1", "C5").array, Some(RangeRef::parse("C5:C6").unwrap())); + run(&mut s, "home.insertRows", json!({"rows": "3:3"})); + let f = formula(&s, "Sheet1", "C6"); + assert_eq!(f.text, "B1:B2*2"); + assert_eq!(f.array, Some(RangeRef::parse("C6:C7").unwrap())); +} + +#[test] +fn deleting_an_unrelated_sheet_keeps_legacy_arrays() { + let mut s = workbook(); + run(&mut s, "range.setValues", json!({"range": "B1", "values": [[1], [2]]})); + run(&mut s, "selection.set", json!({"range": "C1:C2"})); + run(&mut s, "cell.set", json!({"cell": "C1", "input": "=B1:B2*2", "array": true})); + run(&mut s, "home.deleteSheet", json!({"sheet": "Spare"})); + assert_eq!(formula(&s, "Sheet1", "C1").array, Some(RangeRef::parse("C1:C2").unwrap())); +}