diff --git a/crates/engine/src/macros/run.rs b/crates/engine/src/macros/run.rs index 089823973..79dc4c474 100644 --- a/crates/engine/src/macros/run.rs +++ b/crates/engine/src/macros/run.rs @@ -520,7 +520,8 @@ impl Guard { if Arc::ptr_eq(ps, sa) { continue; } - let in_input = |c: CellRef| inputs.iter().any(|(_, p, _)| p.sheet == si && p.range.contains(c)); + // The inputs have followed their cells, so they are on the sheet's index now. + let in_input = |c: CellRef| inputs.iter().any(|(_, p, _)| p.sheet == ai && p.range.contains(c)); let mut hit: Vec = ps .cells .diff(&sa.cells) @@ -1532,6 +1533,27 @@ fn conflict_rule(p: &Json, m: &Macro) -> Result { Ok(rule) } +/// For `macro.check`, as the run would: shifts the data, checks the footprint again, and puts +/// everything back. What still stands in the way, or `None`. +fn shifted_conflicts(s: &mut Session, fp: &Footprint, p: &Prepared, budget: &Machine, down: bool) -> Result> { + let saved = Saved::take(s, p.doc); + let token = s.group_begin("Check Macro")?; + let mut spent = budget.clone(); + (spent.executed, spent.param_bytes) = (fp.executed, fp.param_bytes); + let again = make_room(s, fp, p, down).and_then(|()| footprint(s, p, &spent)); + let rolled = s.group_rollback(token); + saved.restore(s, p.doc); + rolled?; + Ok(match again { + Err(e) => Some(e.to_string()), + Ok(again) => match (&again.error, again.conflict_count()) { + (Some(e), _) => Some(e.describe()), + (None, 0) => None, + (None, _) => Some(format!("Shifting the data doesn't make room ({}).", again.describe_conflicts())), + }, + }) +} + /// `macro.check`: what running the macro would do, without changing anything. pub fn check(s: &mut Session, name: &str, p: &Json) -> Result { let d = s.doc()?; @@ -1593,7 +1615,10 @@ pub fn check(s: &mut Session, name: &str, p: &Json) -> Result { OnConflict::Fail => problems.push(format!("It would write over cells that hold data ({what}).")), OnConflict::Overwrite => {} OnConflict::ShiftDown | OnConflict::ShiftRight => { - if let Some(why) = shift_refusal(&s.doc()?.wb, &fp, &prepared, rule == OnConflict::ShiftDown) { + let down = rule == OnConflict::ShiftDown; + if let Some(why) = shift_refusal(&s.doc()?.wb, &fp, &prepared, down) { + problems.push(why); + } else if let Some(why) = shifted_conflicts(s, &fp, &prepared, &budget, down)? { problems.push(why); } } diff --git a/crates/engine/src/tests/macros.rs b/crates/engine/src/tests/macros.rs index c90acd9c9..0b3fbe34c 100644 --- a/crates/engine/src/tests/macros.rs +++ b/crates/engine/src/tests/macros.rs @@ -205,10 +205,21 @@ fn inputs_and_the_output_follow_their_cells_when_a_step_moves_them() { {"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})); + put( + &mut s, + json!({"name": "Front", "inputs": [{"name": "Input", "kind": "cell"}], "output": {"onConflict": "overwrite"}, "steps": steps.clone()}), + ); s.execute("macro.run", json!({"name": "Front", "inputs": "Sheet1!B2"})).unwrap(); assert_eq!((on(&s, "Sheet1", "B2"), on(&s, "Summary", "B2")), ("x".into(), "".into())); + // Data in the input: the step writes into its own input, which isn't data written over, on + // the input's sheet wherever it moved. + let mut s = super::s(); + set(&mut s, "B2", "held"); + put(&mut s, json!({"name": "Front", "inputs": [{"name": "Input", "kind": "cell"}], "output": {"onConflict": "fail"}, "steps": steps})); + s.execute("macro.run", json!({"name": "Front", "inputs": "Sheet1!B2"})).unwrap(); + assert_eq!(on(&s, "Sheet1", "B2"), "x"); + // Rows inserted above: the input and the output move down with their cells. let mut s = super::s(); set(&mut s, "B5", "in"); @@ -293,6 +304,19 @@ fn shifting_makes_room_and_is_checked_again() { let err = s.execute("macro.run", json!({"name": "Named"})).unwrap_err().to_string(); assert!(err.contains("shifting the data didn't make room (E3 on Sheet1)"), "{err}"); assert!(Arc::ptr_eq(&s.doc().unwrap().wb, &start)); + // The check shifts and checks again as the run does, and puts everything back. + let journal = s.journal.len(); + let c = s.execute("macro.check", json!({"name": "Named"})).unwrap(); + assert_eq!((&c["ok"], &c["errors"]), (&json!(false), &json!(["Shifting the data doesn't make room (E3 on Sheet1)."])), "{c}"); + assert!(Arc::ptr_eq(&s.doc().unwrap().wb, &start)); + assert_eq!(s.journal.len(), journal); + // Where the shift does make room, the check says the run would go ahead. + let mut s = super::s(); + data(&mut s); + set(&mut s, "E2", "old 1"); + put(&mut s, totals()); + let c = s.execute("macro.check", json!({"name": "Totals", "inputs": "B2:C5", "onConflict": "shiftDown"})).unwrap(); + assert_eq!((&c["ok"], c.get("errors")), (&json!(true), None), "{c}"); } #[test] diff --git a/docs/macros.md b/docs/macros.md index 5b9d9be00..d88f940e0 100644 --- a/docs/macros.md +++ b/docs/macros.md @@ -10,7 +10,10 @@ and does not touch files without permission. Agents run macros with `macro.run` like any other command. This page describes the macro format, its tokens, and running, checking and debugging macros. -The engine has no macro recorder or management commands in this version, and no macro interface. +`macro.run` checks the macro, its inputs, its parameters and the policy before it runs, works +out where it writes, and checks each step as it runs; `macro.check` reports what a run would do +without running it. The engine has no macro recorder or management commands in this version, +and no macro interface. ## A macro @@ -123,8 +126,9 @@ refused rather than read as the header. or objects that held data when the run started stops the run and rolls the workbook back (a file already written stays): it asks (`confirm: …`) under `ask` and fails otherwise, since shifting can't make room that late. Moving cells that hold data, as inserting or deleting rows - or cells above them does, counts as writing over them, in the footprint and in this check: - both compare cells by place. + or cells above them does, counts as writing over them, in the footprint and in this check, and + so does a formula whose references such a move rewrites: both compare cells by place, so they + may ask about data that only moved, never the other way. 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). The message ends "Nothing was changed.", or names the files the run already wrote, @@ -139,7 +143,8 @@ refused rather than read as the header. output anchor, any approval needed and the footprint, and puts back everything the dry run changed. `ok` says whether `macro.run` with the same parameters would go ahead without a question; when it wouldn't, `errors` says why: the run would fail at a step, a Stop step rolls it back, it -would write over data under `ask` or `fail`, shifting can't make room, or the run would go past +would write over data under `ask` or `fail`, shifting can't make room (the check shifts and +checks again, then puts everything back), or the run would go past the step or parameter limit counting the dry runs before it. The random numbers RAND gives move on, as after any recalculation.