Merge pull request #3591 from vangel76/perf/engine-adjust-if-changed

Structural edits rewrite only the formulas they change
This commit is contained in:
FredApps
2026-11-08 12:03:48 +01:00
committed by GitHub
4 changed files with 106 additions and 22 deletions
+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()));
}