Engine: a macro's inputs, output and Repeat ranges follow their cells when a step moves them

Inputs, the output and a Repeat's range were bound once, by sheet index and coordinates. A step
that inserted a sheet in front of an input's sheet sent every later token to the new sheet, and
rows inserted above an input left the tokens on the cells that moved into its place; the run
wrote into the wrong cells and reported success.

- Commands record the structural edits they make as they rewrite references (`Ctx::edits`; while
  a macro step runs, `DocState::edits` collects them).
- After each step the machine matches the sheets as the run's guard does and moves the inputs,
  the output and the ranges of the Repeats in progress by those edits, with the formula engine's
  own adjustment.
- A step that deletes an input's or the output's cells, or their sheet, or changes the size of a
  range a Repeat is going over, stops the run, which rolls back.
- Test: `inputs_and_the_output_follow_their_cells_when_a_step_moves_them` (a sheet inserted in
  front, rows inserted above, an input deleted, a Repeat's range grown).
This commit is contained in:
prxnvt
2026-10-25 02:40:21 -04:00
parent 5860857937
commit ad79b071ce
8 changed files with 155 additions and 14 deletions
+1
View File
@@ -1386,6 +1386,7 @@ fn paste_special(s: &mut Session, p: &Json) -> Result<Json> {
gridcraft_formula::adjust::Edit::MoveToSheet { from: src, to_sheet: dest_name, to_row: at.row, to_col: at.col }
};
rewrite_all_formulas(&mut cx.wb, &src_sheet_name, &e);
cx.edits.push((src_sheet_name.clone(), e));
cx.structural = true;
}
let end = at.offset_clamped((tiles_r * h) as i64 - 1, (tiles_c * w) as i64 - 1);
+1 -1
View File
@@ -294,7 +294,7 @@ fn insert_table(s: &mut Session, p: &Json) -> Result<Json> {
let e = Edit::InsertCells { axis: Axis::Rows, range: RangeRef::new(r.start, CellRef::new(r.start.row, r.end.col)) };
shift_cell_features(sh, &e);
let name = sh.name.clone();
rewrite(&mut cx.wb, &name, &e);
rewrite(cx, &name, &e);
let sh = cx.sheet_mut(sheet)?;
for (i, c) in (r.start.col..=r.end.col).enumerate() {
sh.set_value(CellRef::new(r.start.row, c), gridcraft_core::Value::text(format!("Column{}", i + 1)));
+16 -2
View File
@@ -340,6 +340,9 @@ pub struct Ctx<'a> {
/// The command applied sheet protection itself (row/column inserts and deletes the
/// protection allows), so cells it shifts aren't refused as edits of locked cells.
pub protection_checked: bool,
/// The structural edits the command made, in order (the sheet's name, the edit), as
/// references were rewritten for them.
pub edits: Vec<(String, gridcraft_formula::adjust::Edit)>,
}
pub(crate) fn edit<R>(s: &mut Session, f: impl FnOnce(&mut Ctx) -> Result<R>) -> Result<R> {
@@ -357,15 +360,26 @@ pub(crate) fn commit<R>(d: &mut DocState, f: impl FnOnce(&mut Ctx) -> Result<R>)
#[allow(clippy::disallowed_methods)]
let t0 = if cfg!(target_arch = "wasm32") { None } else { std::env::var_os("GRIDCRAFT_PROFILE").map(|_| std::time::Instant::now()) };
let mut sel = d.selection.clone();
let mut ctx = Ctx { wb: (*d.wb).clone(), changed: Vec::new(), structural: false, sel: &mut sel, fit_rows: Vec::new(), protection_checked: false };
let mut ctx = Ctx {
wb: (*d.wb).clone(),
changed: Vec::new(),
structural: false,
sel: &mut sel,
fit_rows: Vec::new(),
protection_checked: false,
edits: Vec::new(),
};
let r = f(&mut ctx)?;
let Ctx { mut wb, changed, structural, mut fit_rows, protection_checked, .. } = ctx;
let Ctx { mut wb, changed, structural, mut fit_rows, protection_checked, edits, .. } = ctx;
if !protection_checked {
check_protection(&d.wb, &wb, &d.unlocked_ranges)?;
}
if !structural {
check_arrays(&d.wb, &wb)?;
}
if let Some(log) = d.edits.as_mut() {
log.extend(edits);
}
if changed.len() <= 200_000 {
fit_rows.extend(changed.iter().map(|(s, c)| (*s, c.row)));
}
+10 -8
View File
@@ -223,7 +223,9 @@ fn shift_sheet_features(sh: &mut Sheet, axis: Axis, at: u32, count: u32, insert:
}
/// Rewrites every formula reference (and chart series) for a structural edit on `target`.
pub(super) fn rewrite(wb: &mut gridcraft_model::Workbook, target: &str, e: &Edit) {
pub(super) fn rewrite(cx: &mut Ctx, target: &str, e: &Edit) {
cx.edits.push((target.to_string(), e.clone()));
let wb = &mut cx.wb;
super::edit::rewrite_all_formulas(wb, target, e);
for si in 0..wb.sheets.len() {
let host = wb.sheets.get(si).map(|s| s.name.clone()).unwrap_or_default();
@@ -307,7 +309,7 @@ fn insert_lines(s: &mut Session, p: &Json, axis: Axis) -> Result<Json> {
}
}
}
rewrite(&mut cx.wb, &name, &Edit::Insert { axis, at, count });
rewrite(cx, &name, &Edit::Insert { axis, at, count });
cx.structural = true;
Ok(Json::Null)
})?;
@@ -372,7 +374,7 @@ fn delete_lines(s: &mut Session, p: &Json, axis: Axis) -> Result<Json> {
}
shift_sheet_features(sh, axis, at, count, false);
}
rewrite(&mut cx.wb, &name, &Edit::Delete { axis, at, count });
rewrite(cx, &name, &Edit::Delete { axis, at, count });
}
cx.structural = true;
Ok(Json::Null)
@@ -424,7 +426,7 @@ fn move_columns(s: &mut Session, p: &Json) -> Result<Json> {
sh.cells.shift_cols(before, count as i64);
shift_sheet_features(sh, Axis::Cols, before, count, true);
}
rewrite(&mut cx.wb, &name, &Edit::Insert { axis: Axis::Cols, at: before, count });
rewrite(cx, &name, &Edit::Insert { axis: Axis::Cols, at: before, count });
let mut touched = Vec::new();
{
@@ -462,14 +464,14 @@ fn move_columns(s: &mut Session, p: &Json) -> Result<Json> {
for cell in touched {
cx.touch(sheet, cell);
}
rewrite(&mut cx.wb, &name, &Edit::Move { from: source, to_row: 0, to_col: destination });
rewrite(cx, &name, &Edit::Move { from: source, to_row: 0, to_col: destination });
{
let sh = cx.sheet_mut(sheet)?;
sh.cells.shift_cols(delete_at + count, -(count as i64));
shift_sheet_features(sh, Axis::Cols, delete_at, count, false);
}
rewrite(&mut cx.wb, &name, &Edit::Delete { axis: Axis::Cols, at: delete_at, count });
rewrite(cx, &name, &Edit::Delete { axis: Axis::Cols, at: delete_at, count });
*cx.sel = crate::Selection {
active: CellRef::new(active_row, final_start),
anchor: CellRef::new(active_row, final_start),
@@ -514,7 +516,7 @@ fn insert_cells(s: &mut Session, p: &Json) -> Result<Json> {
}
shift_cell_features(sh, &e);
let name = cx.wb.sheet(sheet).map(|s| s.name.clone()).unwrap_or_default();
rewrite(&mut cx.wb, &name, &e);
rewrite(cx, &name, &e);
cx.structural = true;
Ok(Json::Null)
})?;
@@ -549,7 +551,7 @@ fn delete_cells(s: &mut Session, p: &Json) -> Result<Json> {
shift_cell_features(sh, &e);
}
// References into the deleted cells become #REF!; the rest move.
rewrite(&mut cx.wb, &name, &e);
rewrite(cx, &name, &e);
cx.structural = true;
Ok(Json::Null)
})?;
+4
View File
@@ -206,6 +206,9 @@ pub struct DocState {
/// workbook is opened from a file, carried by content pasted from such a workbook, and never
/// changed by a trust decision.
pub trust: Trust,
/// While a macro step runs: the structural edits its command made (the sheet's name, the
/// edit), so the macro's inputs, output and Repeat ranges follow their cells as formulas do.
pub edits: Option<Vec<(String, gridcraft_formula::adjust::Edit)>>,
/// The references `edit.goTo` with `record: true` went to in this document, newest first,
/// without names, each area written with its sheet (`Sheet1!B14:C22,Sheet1!F19:G30`).
pub go_to_recent: Vec<String>,
@@ -299,6 +302,7 @@ impl DocState {
content_kind: None,
scripts_trusted,
trust,
edits: None,
go_to_recent: vec![],
}
}
+66 -2
View File
@@ -22,6 +22,7 @@ use std::collections::BTreeSet;
use std::sync::Arc;
use gridcraft_core::{CellRef, RangeRef, Value};
use gridcraft_formula::adjust::Edit;
use gridcraft_model::{Cell, CompareOp, InputKind, Macro, OnConflict, RepeatOf, Sheet, Step, StepKind, Workbook};
use serde::Serialize;
use serde_json::{Value as Json, json};
@@ -645,6 +646,34 @@ impl Machine {
}
}
/// After a step that inserted, deleted or moved sheets, rows, columns or cells, moves the
/// inputs, the output and the ranges of the Repeats in progress with their cells, as the
/// references in formulas move. A step that deletes one of them, or changes the size of a
/// range a Repeat is going over, stops the run.
fn follow(&mut self, before: &Workbook, after: &Workbook, edits: &[(String, Edit)]) -> std::result::Result<(), String> {
let same_sheets = before.sheets.len() == after.sheets.len() && before.sheets.iter().zip(&after.sheets).all(|(b, a)| b.name == a.name);
if same_sheets && edits.is_empty() {
return Ok(());
}
let (pairs, _) = pair_sheets(before, after);
let follow = |what: &str, p: Place| follow_place(before, after, &pairs, edits, p).ok_or_else(|| format!("It deleted the cells of {what}."));
for (name, place, _) in self.inputs.iter_mut() {
*place = follow(name, *place)?;
}
if let Some(o) = self.output {
self.output = Some(follow("the output anchor", o)?);
}
for f in self.frames.iter_mut() {
let Some(l) = f.repeat.as_mut() else { continue };
let over = follow(&format!("the range Repeat {} goes over", f.prefix), l.over)?;
if (over.range.height(), over.range.width()) != (l.over.range.height(), l.over.range.width()) {
return Err(format!("It changed the size of the range Repeat {} goes over.", f.prefix));
}
l.over = over;
}
Ok(())
}
fn env<'a>(&'a self, wb: &'a Workbook) -> Env<'a> {
Env { wb, active_sheet: wb.active_sheet, inputs: &self.inputs, output: self.output, item: self.item() }
}
@@ -670,9 +699,13 @@ impl Machine {
(_, Policy::Allow | Policy::Ask(_)) => {}
}
self.last_params = Some(resolved.clone());
let prev = self.guard.as_ref().and_then(|_| s.docs.iter().find(|d| d.uid == self.doc)).map(|d| d.wb.clone());
let prev = s.docs.iter_mut().find(|d| d.uid == self.doc).map(|d| {
d.edits = Some(Vec::new());
d.wb.clone()
});
let queued = s.ui_requests.len();
let r = s.execute(id, resolved);
let edits = s.docs.iter_mut().find(|d| d.uid == self.doc).and_then(|d| d.edits.take()).unwrap_or_default();
// Macros never open dialogs or web pages, and never start editing a cell; messages for
// the person stay queued.
let mut blocked = None;
@@ -689,8 +722,11 @@ impl Machine {
if let Some(b) = blocked {
return Err(fail(b));
}
let after = s.docs.iter().find(|d| d.uid == self.doc).map(|d| d.wb.clone()).ok_or_else(|| fail("The workbook was closed.".into()))?;
if let Some(prev) = &prev {
self.follow(prev, &after, &edits).map_err(fail)?;
}
if let (Some(g), Some(prev)) = (self.guard.as_mut(), prev) {
let after = s.docs.iter().find(|d| d.uid == self.doc).map(|d| d.wb.clone()).ok_or_else(|| fail("The workbook was closed.".into()))?;
let what = g.check(&prev, &after, &self.inputs);
if !what.is_empty() {
let msg = format!("It would write over data ({what}).");
@@ -701,6 +737,34 @@ impl Machine {
}
}
/// Where the cells of `p` are after a step: on the same sheet wherever it moved (`pairs`, from
/// [`pair_sheets`]), and moved by each structural edit the step made there, or `None` when the
/// step deleted them (or their sheet).
fn follow_place(before: &Workbook, after: &Workbook, pairs: &[(usize, Option<usize>)], edits: &[(String, Edit)], p: Place) -> Option<Place> {
let sheet = pairs.iter().find(|(b, _)| *b == p.sheet).and_then(|(_, a)| *a)?;
let mut name = after.sheet(sheet)?.name.clone();
let old_name = before.sheet(p.sheet).map(|s| s.name.as_str()).unwrap_or_default();
let mut text = format!("{}!{}", resolve::quote_sheet(&name), p.range.a1());
for (target, e) in edits {
if target != &name && target != old_name {
continue;
}
let expr = gridcraft_formula::parse(&text).ok()?;
text = gridcraft_formula::print(&gridcraft_formula::adjust::adjust(expr, target, target, e));
if text.contains("#REF!") {
return None;
}
// A cut and paste can take the cells to another sheet.
let env = Env { wb: after, active_sheet: usize::MAX, inputs: &[], output: None, item: None };
name = after.sheet(resolve::resolve_ref(&text, &env).ok()?.sheet)?.name.clone();
}
let env = Env { wb: after, active_sheet: usize::MAX, inputs: &[], output: None, item: None };
if text.contains('$') {
text = text.replace('$', "");
}
resolve::resolve_ref(&text, &env).ok()
}
/// If's comparison of a cell's value with a step's `value`, the way Excel's `=`, `<`… compare.
fn compare(v: &Value, op: CompareOp, want: &Json) -> bool {
let empty = v.is_empty() || v.as_text() == Some("");
+53
View File
@@ -3,6 +3,7 @@
use std::sync::Arc;
use gridcraft_core::CellRef;
use serde_json::{Value as Json, json};
use super::{s, v};
@@ -176,6 +177,58 @@ fn conflicts_ask_fail_or_overwrite() {
assert_eq!(v(&s, "H1").display(), "400");
}
/// The value of `cell` on the sheet called `sheet`.
fn on(s: &Session, sheet: &str, cell: &str) -> String {
let wb = &s.doc().unwrap().wb;
wb.sheet(wb.sheet_index(sheet).unwrap()).unwrap().value(CellRef::parse(cell).unwrap()).display()
}
#[test]
fn inputs_and_the_output_follow_their_cells_when_a_step_moves_them() {
// A sheet inserted in front: the input is still on Sheet1, not on whatever sheet is first now.
let mut s = s();
let steps = json!([
{"command": "home.insertSheet", "params": {"name": "Summary", "before": 0}},
{"command": "cell.set", "params": {"cell": "Input", "input": "x"}}
]);
put(&mut s, json!({"name": "Front", "inputs": [{"name": "Input", "kind": "cell"}], "output": {"onConflict": "overwrite"}, "steps": steps}));
s.execute("macro.run", json!({"name": "Front", "inputs": "Sheet1!B2"})).unwrap();
assert_eq!((on(&s, "Sheet1", "B2"), on(&s, "Summary", "B2")), ("x".into(), "".into()));
// Rows inserted above: the input and the output move down with their cells.
let mut s = super::s();
set(&mut s, "B5", "in");
let steps = json!([
{"command": "home.insertRows", "params": {"rows": "2:3"}},
{"command": "cell.set", "params": {"cell": "Input", "input": "x"}},
{"command": "cell.set", "params": {"cell": "Output", "input": "y"}}
]);
put(
&mut s,
json!({"name": "Down", "inputs": [{"name": "Input", "kind": "cell"}], "output": {"anchor": "D5", "onConflict": "overwrite"}, "steps": steps}),
);
s.execute("macro.run", json!({"name": "Down", "inputs": "B5"})).unwrap();
assert_eq!((v(&s, "B7").display(), v(&s, "D7").display()), ("x".into(), "y".into()));
assert_eq!((v(&s, "B5").display(), v(&s, "D5").display()), ("".into(), "".into()));
// A step that deletes an input's cells stops the run.
let mut s = super::s();
set(&mut s, "B5", "in");
let steps = json!([{"command": "home.deleteRows", "params": {"rows": "5"}}, {"command": "cell.set", "params": {"cell": "Input", "input": "x"}}]);
put(&mut s, json!({"name": "Gone", "inputs": [{"name": "Input", "kind": "cell"}], "output": {"onConflict": "overwrite"}, "steps": steps}));
let start = s.doc().unwrap().wb.clone();
let err = s.execute("macro.run", json!({"name": "Gone", "inputs": "B5"})).unwrap_err().to_string();
assert!(err.contains("Step 1 (home.deleteRows): It deleted the cells of Input."), "{err}");
assert!(Arc::ptr_eq(&s.doc().unwrap().wb, &start));
// A step inside a Repeat that changes the size of the range it goes over stops the run.
let mut s = super::s();
let body = json!([{"command": "home.insertRows", "params": {"rows": "2"}}]);
put(&mut s, json!({"name": "Grow", "output": {"onConflict": "overwrite"}, "steps": [{"type": "repeat", "over": "A1:A3", "body": body}]}));
let err = s.execute("macro.run", json!({"name": "Grow"})).unwrap_err().to_string();
assert!(err.contains("Step 1.1 (home.insertRows): It changed the size of the range Repeat 1 goes over."), "{err}");
}
#[test]
fn each_step_is_checked_for_writes_over_data() {
let dir = std::env::temp_dir().join(format!("gc-macro-guard-{}", std::process::id()));
+4 -1
View File
@@ -117,7 +117,10 @@ refused rather than read as the header.
runs, which this version doesn't work out, so those rules stop the run as `fail` does.
3. **Run.** One undo step, "Run Macro: Totals". Any error rolls the whole run back and names the
step: `Step 3.1 (cell.set): …` (paths are 1-based; `2.1` is inside a Repeat, `4.then.1` inside
an If).
an If). When a step inserts, deletes or moves sheets, rows, columns or cells, the inputs, the
output and the range of a Repeat in progress move with their cells, as references in formulas
do; a step that deletes one of them, or changes the size of a range a Repeat is going over,
stops the run.
## Storage