perf(engine): rewrite only the formulas a structural edit changes

rewrite_all_formulas borrows each tree with Formula::with_expr and calls
adjust_if_changed, so a formula the edit leaves alone is neither copied
nor printed, and a changed one is printed once. A legacy array whose
range moves while its formula stays the same still gets the new range.

Sheet rename and delete compare the adjusted tree with the old one
instead of printed text, and replace only formulas that changed. Delete
no longer rebuilds every formula in the workbook, so unrelated formulas
keep their array range and dynamic-array mark.

A stored non-canonical formula text is no longer re-printed by an edit
that does not change the formula; a test pins this.

Part of #3576

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
vangel76andClaude Opus 5.5 committed 2026-11-08 04:06:21 +01:00
1 parent 7cb0088361
commit 9ab297ced1
4 files changed
+106 -22

No files matched your search

+9 -9
View File
@@ -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));
}
}
+12 -13
View File
@@ -55,11 +55,12 @@ pub(super) fn delete_sheet(s: &mut Session, p: &Json) -> Result<Json> {
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<Json> {
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() {
+1
View File
@@ -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;
@@ -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<Formula> {
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()));
}