From d309c85c3193b1f199bcdeb977997cc4a660c64b Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Sun, 25 Oct 2026 02:52:43 -0400 Subject: [PATCH 1/5] Engine: Stop Action waits while a macro is being debugged `automate.stopAction` rolls back the undo groups open on the workbook. While a macro was being debugged it rolled back the debugger's own group and left the debugger set, pointing at a group that was gone. It is now refused under the debugger's lock with "Finish or stop the macro first", as edits are; `macro.debug {action: "stop"}` ends the run. - Test: `stop_action_waits_while_a_macro_is_being_debugged`. --- crates/engine/src/lib.rs | 5 ++++- crates/engine/src/tests/macros.rs | 16 ++++++++++++++++ docs/macros.md | 5 +++-- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 844494ec5..19e76227a 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -615,7 +615,10 @@ impl Session { // recording, so stopping again after the run keeps what was recorded. let stops_record_actions = id == "automate.recordActions" && (params.get("stop").and_then(Json::as_bool) == Some(true) || self.recording.is_some()); - if (spec.undoable || stops_record_actions) && id != "macro.debug" && locked { + // Stop Action rolls back the open groups, the debugger's among them, and would leave the + // debugger pointing at a group that is gone: stopping the macro is the debugger's own. + let stops_groups = id == "automate.stopAction"; + if (spec.undoable || stops_record_actions || stops_groups) && id != "macro.debug" && locked { return Err(EngineError::Disabled(id.to_string(), macros::debug::BUSY.into())); } let before = self.active().map(|d| (d.uid, d.wb.clone(), d.selection.clone())); diff --git a/crates/engine/src/tests/macros.rs b/crates/engine/src/tests/macros.rs index 0082f783d..7f5adf0f9 100644 --- a/crates/engine/src/tests/macros.rs +++ b/crates/engine/src/tests/macros.rs @@ -295,6 +295,22 @@ fn shifting_makes_room_and_is_checked_again() { assert!(Arc::ptr_eq(&s.doc().unwrap().wb, &start)); } +#[test] +fn stop_action_waits_while_a_macro_is_being_debugged() { + // Stop Action rolls back the open undo groups; the debugger's is one, so it waits for the + // debugger, which stops the macro itself. + let mut s = s(); + data(&mut s); + put(&mut s, totals()); + s.execute("macro.debug", json!({"action": "start", "name": "Totals", "inputs": "B2:C5", "breakpoints": ["1"]})).unwrap(); + let r = s.execute("automate.stopAction", json!({})); + assert!(matches!(&r, Err(EngineError::Disabled(_, why)) if why == crate::macros::debug::BUSY), "{r:?}"); + assert!(s.macro_debug.is_some(), "the debugger is still there"); + s.execute("macro.debug", json!({"action": "stop"})).unwrap(); + assert!(s.macro_debug.is_none()); + s.execute("automate.stopAction", json!({})).unwrap(); +} + #[test] fn closing_the_debugged_workbook_ends_the_debugger() { // `file.close` is not an edit, so the debugger's lock lets it through; the group went with diff --git a/docs/macros.md b/docs/macros.md index a126c80d2..4006096a7 100644 --- a/docs/macros.md +++ b/docs/macros.md @@ -150,8 +150,9 @@ on, as after any recalculation. be given with any action; `breakpointsOnly: true` makes the list replace the steps' own flags. The run stays one undo step; while it is open, edits of the workbooks it touched (its own, and any other its steps changed) are refused with "Finish or stop the macro first", also when another -command makes them (stopping Record Actions saves a script), and other workbooks stay -editable. Each answer gives the next step with its parameters as they would run, the parameters +command makes them (stopping Record Actions saves a script), and so is Automate › Stop Action, +which would roll the run's undo group back under the debugger; `stop` ends the run instead. Other +workbooks stay editable. Each answer gives the next step with its parameters as they would run, the parameters of the step that just ran, and the cells changed so far. ## Storage From 7dad14e77e231d4b72db8b2b0b53c0ba90f56e0e Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Sun, 25 Oct 2026 03:17:42 -0400 Subject: [PATCH 2/5] Engine: the macro debugger locks its workbook to looking around, ends with Stop Action and checks its parameters - `macro.debug` refuses to run as a step of a script or macro, and is never recorded (the recording buffer and Record Actions' journal both leave it out); a debugger whose undo group something else closed locks nothing, and its next action says the run had ended instead of reporting a keep or a rollback. Tests: `the_debugger_runs_on_its_own_and_is_never_recorded`, `a_debugger_whose_group_was_closed_elsewhere_locks_nothing`. - Stop Action is no longer refused under the debugger: it rolls the run back, ends the debugger and names the macro. Test: `stop_action_stops_a_macro_being_debugged` (was `stop_action_waits_while_a_macro_is_being_debugged`). - Between steps the debugged workbook runs only `macro.debug`, queries and an allow-list of commands that look around (selection, sheet changes, Go To, Find, Copy, zoom, Switch Window, Close, Stop Action); everything else is refused, saving and view modes included. The run's active sheet and selection are put back before its next step. Test: `between_steps_only_looking_around_runs_and_the_run_continues_where_it_was`. - Only the top-level `macro.debug` call rolls the run back after a panic; a panic nested in a step is that step's error. Test: `a_panic_inside_a_step_is_that_steps_error_and_the_run_goes_on`. - `keep` and `breakpointsOnly` must be true or false, breakpoints must be step paths of the macro (at most 10,000), and the action is checked before breakpoints change. Test: `debugger_parameters_are_checked_before_anything_changes`. - Answers: `executed` counts the run's own steps (not the footprint's), `shifted` and `wroteFiles` are reported, a rollback's message names the files that stay, `changed` lists deleted and renamed sheets and at most 500 entries with `changedCells`, and a Repeat over whole rows or columns shows the part in use. Tests: `the_debuggers_answers_count_its_own_steps_and_say_what_stays`, `the_debugger_lists_deleted_and_renamed_sheets_and_caps_the_list`, `the_debugger_shows_a_repeat_over_whole_columns_as_the_run_goes_over_them`. --- crates/engine/src/cmd/extra.rs | 24 +- crates/engine/src/cmd/macros.rs | 2 +- crates/engine/src/lib.rs | 33 ++- crates/engine/src/macros/debug.rs | 379 +++++++++++++++++++++++------- crates/engine/src/macros/run.rs | 6 +- crates/engine/src/record.rs | 7 +- crates/engine/src/tests/macros.rs | 278 +++++++++++++++++++++- 7 files changed, 617 insertions(+), 112 deletions(-) diff --git a/crates/engine/src/cmd/extra.rs b/crates/engine/src/cmd/extra.rs index 858dc9372..679728a63 100644 --- a/crates/engine/src/cmd/extra.rs +++ b/crates/engine/src/cmd/extra.rs @@ -217,7 +217,7 @@ pub fn specs() -> Vec { "Stop Action", [], None, - "{} → {stopped: bool}: stops an action left running on the active workbook (an open undo group, such as a debugger's) and puts back the workbook and its undo history as they were when it began", + "{} → {stopped: bool, macro?, message?}: stops an action left running on the active workbook (an open undo group, such as a debugger's) and puts back the workbook and its undo history as they were when it began; a macro being debugged there is stopped (`macro` names it)", has_doc, stop_action ), @@ -978,8 +978,13 @@ fn queries(s: &mut Session, _: &Json) -> Result { fn record(s: &mut Session, p: &Json) -> Result { if bool_param(p, "stop").unwrap_or(false) || s.recording.is_some() { let Some(start) = s.recording.take() else { return Ok(json!({"recording": false})) }; - let cmds: Vec = - s.journal.iter().skip(start).filter(|(id, _)| !id.starts_with("automate.")).map(|(id, p)| json!({"command": id, "params": p})).collect(); + let cmds: Vec = s + .journal + .iter() + .skip(start) + .filter(|(id, _)| !id.starts_with("automate.") && !crate::record::NOT_RECORDED.contains(&id.as_str())) + .map(|(id, p)| json!({"command": id, "params": p})) + .collect(); let name = str_param(p, "name").map(str::to_string).unwrap_or_else(|| format!("Script {}", start)); s.execute("automate.newScript", json!({"name": name, "commands": cmds}))?; return Ok(json!({"recording": false, "saved": name, "steps": cmds.len()})); @@ -1102,7 +1107,7 @@ fn run_script(s: &mut Session, p: &Json) -> Result { } /// Automate › Stop Action: rolls back the undo groups open on the active workbook, so Undo and -/// Redo work there again. +/// Redo work there again. A macro being debugged on that workbook is stopped with them. fn stop_action(s: &mut Session, _: &Json) -> Result { // Inside another command the open groups are that command's own (an action in progress): // a step of it that stopped them would pull the workbook from under the rest of its run. @@ -1110,7 +1115,16 @@ fn stop_action(s: &mut Session, _: &Json) -> Result { return Err(EngineError::Disabled("automate.stopAction".into(), "Stop Action runs on its own, not as a step of a script or macro.".into())); } let uid = s.doc()?.uid; - Ok(json!({"stopped": s.abort_groups(uid)})) + let mut out = json!({"stopped": s.abort_groups(uid)}); + // A macro being debugged here had its group among them: its run is over. + if let Some((name, wrote_files)) = crate::macros::debug::end_stopped(s, uid) { + out["macro"] = json!(name); + out["message"] = json!(format!( + "Macro '{name}', which was being debugged, was stopped and rolled back. {}", + crate::macros::run::rolled_back(&wrote_files) + )); + } + Ok(out) } fn delete_script(s: &mut Session, p: &Json) -> Result { diff --git a/crates/engine/src/cmd/macros.rs b/crates/engine/src/cmd/macros.rs index 76c0134b3..b7e53ec03 100644 --- a/crates/engine/src/cmd/macros.rs +++ b/crates/engine/src/cmd/macros.rs @@ -30,7 +30,7 @@ pub fn specs() -> Vec { "Step Into", [], None, - "{action: start|step|continue|runTo|stop|state, name? and inputs?, onConflict?, approve? (start), step: path (runTo), keep?: bool (stop), breakpoints?: [\"3\", \"2.1\"], breakpointsOnly?: bool (the list replaces the steps' own breakpoints)} debugs a macro one step at a time as one undo step; edits of the workbooks it touched wait until it finishes or stops (other workbooks stay editable) → {state: paused|finished|stopped|failed, current: {step, kind, command?, params?, iteration?}, lastParams, executed, changed, atBreakpoint?, error?}", + "{action: start|step|continue|runTo|stop|state, name, inputs?, onConflict?, overwrite?, approve?, timeLimit?: seconds per action (start, as macro.run), step: path (runTo), keep?: bool (stop), breakpoints?: [\"3\", \"2.1\"] (steps of the macro), breakpointsOnly?: bool (the list replaces the steps' own breakpoints)} debugs a macro one step at a time; stop keeps the changes as one undo step or rolls them back. Runs on its own, never as a step of a script or macro. Until it finishes or stops, its workbook runs only macro.debug, queries, selection and sheet changes, Go To, Find, Copy, zoom, Switch Window, Close and Stop Action (other workbooks stay editable) → {state: paused|finished|stopped|failed, current: {step, kind, command?, params?, iteration?}, lastParams, executed, changed (at most 500), changedCells, atBreakpoint?, shifted?, wroteFiles?, message?, error?}", has_doc, |s, p| crate::macros::debug::debug(s, p) ), diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 19e76227a..d85616c5e 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -607,18 +607,19 @@ impl Session { if let Err(why) = (spec.enabled)(self) { return Err(EngineError::Disabled(id.to_string(), why)); } - // While a macro is being debugged its undo group is open: other edits of the workbooks it - // touched wait, also when another command runs them (a script, or a recording that stops - // and saves). Only the run's own steps go ahead. Other workbooks stay editable. + // A debugger whose undo group something else ended locks nothing any more (`macro.debug` + // itself says that the run had ended). + if id != "macro.debug" { + macros::debug::drop_if_ended(self); + } + let query = !spec.journal && !spec.undoable; + // While a macro is being debugged its undo group is open, and its workbook is locked: + // between steps only the debugger, queries and commands that look around run there + // (`macros::debug::LOOKS_AROUND`). Anything else waits, undoable or not (saving, view + // modes, stopping Record Actions, which saves a script), also when another command runs + // it. Only the run's own steps go ahead. Other workbooks stay editable. let locked = self.active().is_some_and(|d| self.macro_debug.as_ref().is_some_and(|m| m.locks(d.uid) && !m.stepping())); - // Stopping Record Actions saves a script into the workbook; refuse it before it takes the - // recording, so stopping again after the run keeps what was recorded. - let stops_record_actions = - id == "automate.recordActions" && (params.get("stop").and_then(Json::as_bool) == Some(true) || self.recording.is_some()); - // Stop Action rolls back the open groups, the debugger's among them, and would leave the - // debugger pointing at a group that is gone: stopping the macro is the debugger's own. - let stops_groups = id == "automate.stopAction"; - if (spec.undoable || stops_record_actions || stops_groups) && id != "macro.debug" && locked { + if locked && !macros::debug::runs_while_locked(id, query) { return Err(EngineError::Disabled(id.to_string(), macros::debug::BUSY.into())); } let before = self.active().map(|d| (d.uid, d.wb.clone(), d.selection.clone())); @@ -631,7 +632,6 @@ impl Session { // because a nested `edit.undo` removes entries. A query runs nothing that changes the // history (`every_query_leaves_the_history_alone` runs each one), so it needs no // copy; front ends run queries all the time. - let query = !spec.journal && !spec.undoable; let history_before = before.as_ref().filter(|_| !query).and_then(|(uid, ..)| { #[cfg(test)] tests::HISTORY_COPIES.with(|n| n.set(n.get().saturating_add(1))); @@ -689,8 +689,10 @@ impl Session { } d.calc.rebuild(&d.wb); } - macros::debug::recover(self); if self.depth == 0 { + // Only the `macro.debug` call itself can lose the run's place: a command nested + // in a step that panics is an error of that step, which the run handles. + macros::debug::recover(self); self.trim_history(); self.group_step = None; } @@ -970,6 +972,11 @@ impl Session { r } + /// Whether the token's undo group is still open (nothing has ended or rolled it back). + pub fn group_is_open(&self, token: &GroupToken) -> bool { + self.groups.iter().any(|g| g.id == token.id) + } + /// Whether an undo group is open on the document with this uid. pub fn in_group(&self, doc: u64) -> bool { self.groups.iter().any(|g| g.doc == doc) diff --git a/crates/engine/src/macros/debug.rs b/crates/engine/src/macros/debug.rs index 19aa03167..acbc320f3 100644 --- a/crates/engine/src/macros/debug.rs +++ b/crates/engine/src/macros/debug.rs @@ -2,10 +2,11 @@ //! //! `macro.debug` starts a run that pauses before its first step, then steps, continues to the next //! breakpoint, runs to a chosen step, or stops. The run lives on the session between calls (one -//! action per call) inside an undo group that spans them, so a debugged run is still -//! one undo step; Stop keeps the changes or rolls them back. While a macro is being debugged, -//! other edits of the workbooks it touched (its own, and any other its steps changed) are refused -//! ("Finish or stop the macro first"); other workbooks stay editable. +//! action per call) inside an undo group that spans them; Stop keeps the changes as one undo step +//! or rolls them back. While a macro is being debugged its workbook is locked: only the +//! debugger, queries and the commands in [`LOOKS_AROUND`] run there, and everything else, saving +//! included, is refused ("Finish or stop the macro first"). Other workbooks stay editable. The +//! debugger runs on its own, never as a step of a script or macro, and is never recorded. use std::collections::BTreeSet; use std::sync::Arc; @@ -13,8 +14,45 @@ use std::sync::Arc; use gridcraft_model::Workbook; use serde_json::{Value as Json, json}; -use super::run::{Machine, StepError}; -use crate::{EngineError, GroupToken, Result, Session}; +use super::run::{Machine, StepError, rolled_back}; +use crate::{EngineError, GroupToken, Result, Selection, Session}; + +/// The most breakpoints one call sets. +pub const MAX_BREAKPOINTS: usize = 10_000; +/// The most entries `changed` lists (ranges and sheet notes), as the footprint lists ranges. +const MAX_LISTED: usize = 500; + +/// Commands that run on the debugged workbook between steps, besides `macro.debug` and queries. +/// They only look around (the selection, the active sheet, Go To, Find, zoom, copying), switch to +/// another window, close the workbook (which ends the run) or stop the run (Stop Action). The +/// debugger puts the run's active sheet and selection back before its next step runs. +pub const LOOKS_AROUND: &[&str] = &[ + "selection.set", + "selection.move", + "selection.next", + "selection.row", + "selection.column", + "selection.currentRegion", + "edit.selectAll", + "sheet.activate", + "sheet.next", + "sheet.previous", + "edit.goTo", + "edit.goToSpecial", + "edit.find", + "edit.copy", + "view.zoom", + "view.zoom100", + "view.zoomToSelection", + "window.activate", + "file.close", + "automate.stopAction", +]; + +/// Whether command `id` (a query when `query`) runs on a workbook the debugger locks. +pub fn runs_while_locked(id: &str, query: bool) -> bool { + query || id == "macro.debug" || LOOKS_AROUND.contains(&id) +} /// A macro being debugged. pub struct Debugger { @@ -30,11 +68,27 @@ pub struct Debugger { pub only_listed: bool, /// The workbook when the run started, for "changed so far". start: Arc, - /// Every step path in the macro, for checking `runTo` targets. + /// Every step path in the macro, for checking `runTo` targets and breakpoints. paths: BTreeSet, - /// The workbooks the run has touched (by uid): its own, and any other a step changed. Edits - /// of these wait until the run ends. + /// The workbooks the run has touched (by uid). Steps run on the macro's own workbook and the + /// policy refuses window and file commands, so this is the macro's workbook unless a command + /// reaches another one some other way; edits of these wait until the run ends. touched: BTreeSet, + /// Steps the footprint's dry runs executed before the run (they count towards the run's + /// budget, not towards `executed`). + base: u64, + /// The conflict rule that shifted the data at the start (`shiftDown`, `shiftRight`). + shifted: Option<&'static str>, + /// Where the run left its workbook after the last action. + view: Option, +} + +/// The active sheet and the selections of the debugged workbook where the run left them. +#[derive(Clone)] +struct View { + sheet: usize, + selection: Selection, + sheet_selections: Vec, } impl Debugger { @@ -47,6 +101,11 @@ impl Debugger { pub fn stepping(&self) -> bool { self.machine.is_none() } + + /// The run's own steps so far. + fn executed(&self) -> u64 { + self.machine.as_ref().map_or(0, |m| m.executed.saturating_sub(self.base)) + } } /// Each open workbook's uid and its current version, to see which ones a step changes. @@ -63,52 +122,131 @@ fn note_touched(s: &mut Session, before: &[(u64, Arc)]) { } } +/// Remembers where the run left its workbook: the active sheet and the selections. +fn remember_view(s: &mut Session) { + let Some(uid) = s.macro_debug.as_ref().map(|d| d.doc) else { return }; + let view = s.docs.iter().find(|d| d.uid == uid).map(|d| View { + sheet: d.wb.active_sheet, + selection: d.selection.clone(), + sheet_selections: d.sheet_selections.clone(), + }); + if let Some(dbg) = s.macro_debug.as_mut() { + dbg.view = view; + } +} + +/// Puts back the active sheet and the selections where the run left them, so the next step runs +/// where `macro.run` would run it, however the person looked around meanwhile. +fn put_back_view(s: &mut Session) { + let Some((uid, Some(view))) = s.macro_debug.as_ref().map(|d| (d.doc, d.view.clone())) else { return }; + let Some(d) = s.docs.iter_mut().find(|d| d.uid == uid) else { return }; + if d.wb.active_sheet != view.sheet && view.sheet < d.wb.sheets.len() { + // The active sheet is view state: change it without making the workbook dirty. + let dirty = d.is_dirty(); + Arc::make_mut(&mut d.wb).active_sheet = view.sheet; + if !dirty { + d.saved = d.wb.clone(); + } + } + d.selection = view.selection; + d.sheet_selections = view.sheet_selections; +} + /// The message for edits refused while a macro is being debugged. pub const BUSY: &str = "Finish or stop the macro first"; +/// Ends a debugger whose undo group is no longer open (something other than the debugger ended +/// or rolled it back): its workbook is not locked any more. Returns the macro's name when it did. +pub(crate) fn drop_if_ended(s: &mut Session) -> Option { + let d = s.macro_debug.as_ref()?; + if d.stepping() || d.token.as_ref().is_some_and(|t| s.group_is_open(t)) { + return None; + } + s.macro_debug.take().map(|d| d.name) +} + +/// After Stop Action rolled back the undo groups of the workbook with uid `doc`: ends the +/// debugger if its run was there. Returns the macro's name and the files the run wrote, which stay. +pub(crate) fn end_stopped(s: &mut Session, doc: u64) -> Option<(String, Vec)> { + if !s.macro_debug.as_ref().is_some_and(|d| d.doc == doc && !d.stepping()) { + return None; + } + let d = s.macro_debug.take()?; + Some((d.name, d.machine.map(|m| m.wrote_files).unwrap_or_default())) +} + +/// Why an action finds the run already ended: its undo group was closed outside the debugger. +fn already_ended(name: &str) -> String { + format!("Macro '{name}' was no longer being debugged: its undo group had been closed outside the debugger, so nothing was kept or rolled back.") +} + enum Mode { Step, Continue, RunTo(String), } +const ACTIONS: [&str; 6] = ["start", "step", "continue", "runTo", "stop", "state"]; + /// `macro.debug {name?, inputs?, action: start|step|continue|runTo|stop|state, step?, keep?, -/// breakpoints?}`. +/// breakpoints?, breakpointsOnly?}`. pub fn debug(s: &mut Session, p: &Json) -> Result { + // A step of a script or macro that started the debugger would leave it behind when the + // script fails and rolls back its groups, the debugger's among them. + if s.depth() > 1 { + return Err(EngineError::Disabled("macro.debug".into(), "The macro debugger runs on its own, not as a step of a script or macro.".into())); + } + let ended = drop_if_ended(s); let action = match p.get("action") { + None | Some(Json::Null) if s.macro_debug.is_some() || ended.is_some() => "state".to_string(), + None | Some(Json::Null) => "start".to_string(), Some(Json::String(a)) => a.clone(), // `{"action": {"runTo": {"step": "3"}}}` - Some(Json::Object(o)) => o.keys().next().cloned().unwrap_or_default(), - _ if s.macro_debug.is_some() => "state".into(), - _ => "start".into(), + Some(Json::Object(o)) if o.len() == 1 => o.keys().next().cloned().unwrap_or_default(), + Some(_) => return Err(bad("`action` must be start, step, continue, runTo, stop or state")), }; - if action != "start" - && let (Some(d), Some(bps)) = (s.macro_debug.as_mut(), p.get("breakpoints")) + if !ACTIONS.contains(&action.as_str()) { + return Err(bad(format!("action must be start, step, continue, runTo, stop or state, not '{action}'"))); + } + if let Some(name) = ended + && action != "start" { - d.breakpoints = paths_param(bps)?; - if let Some(only) = p.get("breakpointsOnly").and_then(Json::as_bool) { + return Err(EngineError::Other(already_ended(&name))); + } + let only = bool_param(p, "breakpointsOnly")?; + if action == "start" { + return start(s, p, only.unwrap_or(false)); + } + let keep = bool_param(p, "keep")?; + let d = s.macro_debug.as_ref().ok_or_else(not_debugging)?; + let target = if action == "runTo" { + let target = p + .get("step") + .or_else(|| p.pointer("/action/runTo/step")) + .and_then(path_of) + .ok_or_else(|| bad("`runTo` needs `step`: the path of the step to stop at"))?; + if !d.paths.contains(&target) { + return Err(bad(format!("macro '{}' has no step {target}", d.name))); + } + Some(target) + } else { + None + }; + let breakpoints = p.get("breakpoints").filter(|v| !v.is_null()).map(|v| paths_param(v, &d.paths, &d.name)).transpose()?; + if let Some(d) = s.macro_debug.as_mut() { + if let Some(bps) = breakpoints { + d.breakpoints = bps; + } + if let Some(only) = only { d.only_listed = only; } } - match action.as_str() { - "start" => start(s, p), - "step" => advance(s, Mode::Step), - "continue" => advance(s, Mode::Continue), - "runTo" => { - let target = p - .get("step") - .or_else(|| p.pointer("/action/runTo/step")) - .and_then(path_of) - .ok_or_else(|| bad("`runTo` needs `step`: the path of the step to stop at"))?; - let d = s.macro_debug.as_ref().ok_or_else(not_debugging)?; - if !d.paths.contains(&target) { - return Err(bad(format!("macro '{}' has no step {target}", d.name))); - } - advance(s, Mode::RunTo(target)) - } - "stop" => stop(s, p.get("keep").and_then(Json::as_bool).unwrap_or(false)), - "state" => state(s), - other => Err(bad(format!("action must be start, step, continue, runTo, stop or state, not '{other}'"))), + match (action.as_str(), target) { + ("step", _) => advance(s, Mode::Step), + ("continue", _) => advance(s, Mode::Continue), + ("runTo", Some(target)) => advance(s, Mode::RunTo(target)), + ("stop", _) => stop(s, keep.unwrap_or(false)), + _ => state(s), } } @@ -120,6 +258,15 @@ fn not_debugging() -> EngineError { EngineError::Other("No macro is being debugged. Start one with action: start.".into()) } +/// `true`, `false`, or not given (`null` counts as not given); anything else is refused. +fn bool_param(p: &Json, key: &str) -> Result> { + match p.get(key) { + None | Some(Json::Null) => Ok(None), + Some(Json::Bool(b)) => Ok(Some(*b)), + Some(_) => Err(bad(format!("`{key}` must be true or false"))), + } +} + /// A step path from JSON: `"2.1"` or `3`. fn path_of(v: &Json) -> Option { match v { @@ -129,47 +276,66 @@ fn path_of(v: &Json) -> Option { } } -fn paths_param(v: &Json) -> Result> { +/// The `breakpoints` list: step paths of macro `name` (one of `paths`), at most +/// [`MAX_BREAKPOINTS`]. +fn paths_param(v: &Json, paths: &BTreeSet, name: &str) -> Result> { let list = v.as_array().ok_or_else(|| bad("`breakpoints` must be a list of step paths"))?; - list.iter().take(10_000).map(|x| path_of(x).ok_or_else(|| bad("a breakpoint is a step path such as \"3\" or \"2.1\""))).collect() + if list.len() > MAX_BREAKPOINTS { + return Err(bad(format!("`breakpoints` lists {} steps; at most {MAX_BREAKPOINTS} can be set", list.len()))); + } + let mut out = BTreeSet::new(); + for x in list { + let path = path_of(x).ok_or_else(|| bad("a breakpoint is a step path such as \"3\" or \"2.1\""))?; + if !paths.contains(&path) { + return Err(bad(format!("macro '{name}' has no step {path} for a breakpoint"))); + } + out.insert(path); + } + Ok(out) } -fn start(s: &mut Session, p: &Json) -> Result { +fn start(s: &mut Session, p: &Json, only_listed: bool) -> Result { if let Some(d) = &s.macro_debug { return Err(EngineError::Other(format!("Macro '{}' is being debugged. Stop it first.", d.name))); } let name = p.get("name").and_then(Json::as_str).filter(|n| !n.trim().is_empty()).ok_or_else(|| bad("missing `name`"))?; let b = super::run::begin(s, name, p)?; - let breakpoints = match p.get("breakpoints") { - Some(v) => paths_param(v)?, + let paths = all_paths(&b.mac.steps, ""); + let breakpoints = match p.get("breakpoints").filter(|v| !v.is_null()) { + Some(v) => paths_param(v, &paths, &b.mac.name)?, None => BTreeSet::new(), }; let start_wb = s.doc()?.wb.clone(); let token = s.group_begin(&format!("Run Macro: {}", b.mac.name))?; // The run's machine: unless the rule is overwrite, it checks each step for writes over data. - let mut machine = match super::run::make_way(s, &b) { - Ok((machine, _)) => machine, + let (mut machine, shifted) = match super::run::make_way(s, &b) { + Ok(made) => made, Err(e) => { let _ = s.group_rollback(token); return Err(e); } }; + // Each action has the time limit to itself; the dry runs just had theirs. machine.restart_clock(); - let paths = all_paths(&b.mac.steps, ""); + let base = machine.executed; s.macro_debug = Some(Debugger { name: b.mac.name.clone(), doc: b.prepared.doc, machine: Some(machine), token: Some(token), breakpoints, - only_listed: p.get("breakpointsOnly").and_then(Json::as_bool).unwrap_or(false), + only_listed, start: start_wb, paths, touched: BTreeSet::from([b.prepared.doc]), + base, + shifted: shifted.then(|| b.rule.name()), + view: None, }); if s.macro_debug.as_ref().and_then(|d| d.machine.as_ref()).is_some_and(Machine::is_done) { return finish(s, None); } + remember_view(s); state(s) } @@ -192,8 +358,9 @@ fn all_paths(steps: &[gridcraft_model::Step], prefix: &str) -> BTreeSet /// Runs one step (`Step`), or steps until a breakpoint, the target step or the end. fn advance(s: &mut Session, mode: Mode) -> Result { + put_back_view(s); let d = s.macro_debug.as_mut().ok_or_else(not_debugging)?; - let name = d.name.clone(); + let (name, base) = (d.name.clone(), d.base); let breakpoints = d.breakpoints.clone(); let only_listed = d.only_listed; let mut machine = d.machine.take().ok_or_else(not_debugging)?; @@ -225,13 +392,12 @@ fn advance(s: &mut Session, mode: Mode) -> Result { } } let done = machine.is_done(); - let executed = machine.executed; match s.macro_debug.as_mut() { Some(d) => d.machine = Some(machine), - // A step panicked and `recover` has already rolled the run back. + // The macro.debug call panicked and `recover` has already rolled the run back. None => { return match outcome { - Err(e) => Ok(failed(&name, executed, &e)), + Err(e) => Ok(failed(&name, machine.executed.saturating_sub(base), &e)), Ok(()) => Err(not_debugging()), }; } @@ -243,23 +409,41 @@ fn advance(s: &mut Session, mode: Mode) -> Result { if done { return finish(s, None); } + remember_view(s); let mut out = state(s)?; out["atBreakpoint"] = json!(hit); Ok(out) } +/// What every answer about the run's end or its state adds: the data shift at the start and the +/// files the run wrote (they stay, whatever happens to the workbook). +fn add_notes(out: &mut Json, d: &Debugger, wrote_files: &[String]) { + if let Some(rule) = d.shifted { + out["shifted"] = json!(rule); + } + if !wrote_files.is_empty() { + out["wroteFiles"] = json!(wrote_files); + } +} + /// Ends the run after its last step (kept as one undo step, unless a Stop step asked for a /// rollback) or a failed step (rolled back). fn finish(s: &mut Session, error: Option) -> Result { let Some(mut d) = s.macro_debug.take() else { return Err(not_debugging()) }; + let (changed, cells) = changed(s, &d); + let executed = d.executed(); let machine = d.machine.take(); - let changed = changed(s, &d); let stopped = machine.as_ref().and_then(|m| m.stopped.clone()); - let executed = machine.as_ref().map_or(0, |m| m.executed); + let wrote = machine.map(|m| m.wrote_files).unwrap_or_default(); let rollback = error.is_some() || matches!(stopped, Some((_, true))); - if let Some(token) = d.token.take() { - // The group is gone only if its workbook was closed; then there is nothing to end. - let _ = if rollback { s.group_rollback(token) } else { s.group_end(token).map(|_| ()) }; + let ended = match d.token.take() { + Some(token) if rollback => s.group_rollback(token).is_ok(), + Some(token) => s.group_end(token).is_ok(), + None => false, + }; + if !ended { + let step = error.map(|e| format!("{} ", e.describe())).unwrap_or_default(); + return Err(EngineError::Other(format!("{step}{}", already_ended(&d.name)))); } if let Some(e) = &error { return Ok(failed(&d.name, executed, e)); @@ -270,79 +454,114 @@ fn finish(s: &mut Session, error: Option) -> Result { out["state"] = json!("stopped"); out["stopped"] = json!(msg); out["kept"] = json!(!rolled); + if *rolled { + out["message"] = json!(rolled_back(&wrote)); + } } None => out["state"] = json!("finished"), } if !rollback { out["changed"] = json!(changed); + out["changedCells"] = json!(cells); } + add_notes(&mut out, &d, &wrote); Ok(out) } /// The result of a run that a failed step rolled back. fn failed(name: &str, executed: u64, e: &StepError) -> Json { - json!({ + let mut out = json!({ "macro": name, "executed": executed, "state": "failed", "error": serde_json::to_value(e).unwrap_or(Json::Null), - "message": format!("{} Nothing was changed.", e.describe()), - }) + "message": format!("{} {}", e.describe(), rolled_back(&e.wrote_files)), + }); + if !e.wrote_files.is_empty() { + out["wroteFiles"] = json!(e.wrote_files); + } + out } /// Stops the run: keeps what it did as one undo step, or rolls it back. fn stop(s: &mut Session, keep: bool) -> Result { let Some(mut d) = s.macro_debug.take() else { return Err(not_debugging()) }; - let changed = changed(s, &d); - if let Some(token) = d.token.take() { - let _ = if keep { s.group_end(token).map(|_| ()) } else { s.group_rollback(token) }; + let (changed, cells) = changed(s, &d); + let ended = match d.token.take() { + Some(token) if keep => s.group_end(token).is_ok(), + Some(token) => s.group_rollback(token).is_ok(), + None => false, + }; + if !ended { + return Err(EngineError::Other(already_ended(&d.name))); } - let executed = d.machine.as_ref().map_or(0, |m| m.executed); - let mut out = json!({"state": "stopped", "macro": d.name, "kept": keep, "executed": executed}); + let wrote = d.machine.as_ref().map(|m| m.wrote_files.clone()).unwrap_or_default(); + let mut out = json!({"state": "stopped", "macro": d.name, "kept": keep, "executed": d.executed()}); if keep { out["changed"] = json!(changed); + out["changedCells"] = json!(cells); + } else { + out["message"] = json!(rolled_back(&wrote)); } + add_notes(&mut out, &d, &wrote); Ok(out) } /// Where the run is: the next step with its parameters as they would run, the step that just -/// ran, and the cells changed so far. +/// ran, and what the run has changed in its workbook so far. fn state(s: &mut Session) -> Result { let d = s.macro_debug.as_ref().ok_or_else(not_debugging)?; let machine = d.machine.as_ref().ok_or_else(not_debugging)?; let wb = s.docs.iter().find(|x| x.uid == d.doc).map(|x| x.wb.clone()).ok_or(EngineError::NoDocument)?; - Ok(json!({ + let (changed, cells) = changed(s, d); + let mut out = json!({ "state": "paused", "macro": d.name, "current": machine.current_json(&wb), "lastParams": machine.last_params, - "executed": machine.executed, + "executed": d.executed(), "breakpoints": d.breakpoints, - "changed": changed(s, d), - })) + "changed": changed, + "changedCells": cells, + }); + add_notes(&mut out, d, &machine.wrote_files); + Ok(out) } -/// The cells changed since the run started, as `Sheet!A1:B2` ranges. -fn changed(s: &Session, d: &Debugger) -> Vec { - let Some(now) = s.docs.iter().find(|x| x.uid == d.doc).map(|x| x.wb.clone()) else { return vec![] }; +/// What the run changed in its workbook since it started: cells as `Sheet!A1:B2` ranges and the +/// sheets it added, deleted or renamed (at most [`MAX_LISTED`] entries), and how many cells +/// changed on the sheets the workbook had at the start. +fn changed(s: &Session, d: &Debugger) -> (Vec, usize) { + let Some(now) = s.docs.iter().find(|x| x.uid == d.doc).map(|x| x.wb.clone()) else { return (vec![], 0) }; + let (pairs, added) = super::run::pair_sheets(&d.start, &now); let mut out = Vec::new(); - for sh in &now.sheets { - let Some(before) = d.start.sheet_index(&sh.name).and_then(|i| d.start.sheets.get(i)) else { - out.push(format!("{} (new sheet)", sh.name)); + let mut count = 0usize; + for (bi, ai) in pairs { + let Some(before) = d.start.sheets.get(bi) else { continue }; + let Some(after) = ai.and_then(|a| now.sheets.get(a)) else { + out.push(format!("{} (deleted sheet)", before.name)); continue; }; - if Arc::ptr_eq(before, sh) { + if before.name != after.name { + out.push(format!("{} (renamed from {})", after.name, before.name)); + } + if Arc::ptr_eq(before, after) { continue; } - let mut cells = before.cells.diff(&sh.cells); - let q = super::resolve::quote_sheet(&sh.name); - out.extend(super::run::ranges(&mut cells).into_iter().map(|r| format!("{q}!{r}"))); + let mut cells = before.cells.diff(&after.cells); + count = count.saturating_add(cells.len()); + if out.len() < MAX_LISTED { + let q = super::resolve::quote_sheet(&after.name); + out.extend(super::run::ranges(&mut cells).into_iter().map(|r| format!("{q}!{r}"))); + } } - out + out.extend(added.into_iter().filter_map(|a| now.sheets.get(a)).map(|sh| format!("{} (new sheet)", sh.name))); + out.truncate(MAX_LISTED); + (out, count) } -/// After a command panicked while a macro was being debugged: if the run lost its place, roll -/// it back so its undo group doesn't stay open. +/// After the `macro.debug` call itself panicked: if the run lost its place, roll it back so its +/// undo group doesn't stay open. pub(crate) fn recover(s: &mut Session) { if s.macro_debug.as_ref().is_some_and(Debugger::stepping) && let Some(mut d) = s.macro_debug.take() diff --git a/crates/engine/src/macros/run.rs b/crates/engine/src/macros/run.rs index 337acc23d..089823973 100644 --- a/crates/engine/src/macros/run.rs +++ b/crates/engine/src/macros/run.rs @@ -631,7 +631,9 @@ impl Machine { } StepKind::Repeat { over, of, .. } => { out["kind"] = json!("repeat"); - out["over"] = json!(resolve::resolve_ref(over, &env).map(|p| resolve::a1(p, &env)).unwrap_or_else(|e| e.message().to_string())); + // Whole rows or columns shrink to the part in use, as when the Repeat runs. + out["over"] = + json!(resolve::resolve_ref(over, &env).map(|p| resolve::a1(trim(wb, p), &env)).unwrap_or_else(|e| e.message().to_string())); out["of"] = json!(if *of == RepeatOf::Rows { "rows" } else { "columns" }); } StepKind::If { cell, .. } => { @@ -1217,7 +1219,7 @@ fn diff(before: &Workbook, after: &Workbook, inputs: &[Bound], touched: &[(Strin /// Each sheet of `before` with its index in `after` (`None` when deleted), and the sheets of /// `after` that are new. Sheets are matched by name; what's left is matched in order (a renamed /// sheet). -fn pair_sheets(before: &Workbook, after: &Workbook) -> (Vec<(usize, Option)>, Vec) { +pub(crate) fn pair_sheets(before: &Workbook, after: &Workbook) -> (Vec<(usize, Option)>, Vec) { let mut pairs: Vec<(usize, Option)> = before.sheets.iter().enumerate().map(|(bi, sh)| (bi, after.sheet_index(&sh.name))).collect(); let matched: BTreeSet = pairs.iter().filter_map(|(_, a)| *a).collect(); let mut spare = (0..after.sheets.len()).filter(|a| !matched.contains(a)); diff --git a/crates/engine/src/record.rs b/crates/engine/src/record.rs index 4a5d7c4c9..67dcb1cfb 100644 --- a/crates/engine/src/record.rs +++ b/crates/engine/src/record.rs @@ -18,9 +18,10 @@ pub const RECORD_LIMIT: usize = 100_000; /// without a size cap a recording of a few opens of a large workbook keeps tens of MB alive. pub const RECORD_MAX_BYTES: usize = 64 * 1024 * 1024; -/// Commands never recorded: recovery actions, not steps of the person's work. A recording that -/// replayed Stop Action would stop the run it is a step of. -pub const NOT_RECORDED: [&str; 1] = ["automate.stopAction"]; +/// Commands never recorded: recovery actions and the debugger, not steps of the person's work. A +/// recording that replayed Stop Action would stop the run it is a step of, and the debugger runs +/// on its own, never as a step of a script or macro. +pub const NOT_RECORDED: [&str; 2] = ["automate.stopAction", "macro.debug"]; /// One recorded command and where it ran. #[derive(Clone, Debug, PartialEq, Serialize)] diff --git a/crates/engine/src/tests/macros.rs b/crates/engine/src/tests/macros.rs index 7f5adf0f9..c90acd9c9 100644 --- a/crates/engine/src/tests/macros.rs +++ b/crates/engine/src/tests/macros.rs @@ -296,19 +296,281 @@ fn shifting_makes_room_and_is_checked_again() { } #[test] -fn stop_action_waits_while_a_macro_is_being_debugged() { - // Stop Action rolls back the open undo groups; the debugger's is one, so it waits for the - // debugger, which stops the macro itself. +fn stop_action_stops_a_macro_being_debugged() { + // Stop Action rolls back the open undo groups, the debugger's among them; the run ends with + // its group, and the answer says so. let mut s = s(); data(&mut s); put(&mut s, totals()); + let (start, undo) = (s.doc().unwrap().wb.clone(), undo_labels(&s)); s.execute("macro.debug", json!({"action": "start", "name": "Totals", "inputs": "B2:C5", "breakpoints": ["1"]})).unwrap(); - let r = s.execute("automate.stopAction", json!({})); - assert!(matches!(&r, Err(EngineError::Disabled(_, why)) if why == crate::macros::debug::BUSY), "{r:?}"); - assert!(s.macro_debug.is_some(), "the debugger is still there"); - s.execute("macro.debug", json!({"action": "stop"})).unwrap(); + s.execute("macro.debug", json!({"action": "step"})).unwrap(); + assert_eq!(v(&s, "E2").display(), "Total"); + let r = s.execute("automate.stopAction", json!({})).unwrap(); + assert_eq!((r["stopped"].as_bool(), r["macro"].as_str()), (Some(true), Some("Totals")), "{r}"); + assert!(r["message"].as_str().unwrap().contains("was stopped and rolled back"), "{r}"); + assert!(s.macro_debug.is_none(), "the debugger ended with its group"); + assert!(Arc::ptr_eq(&s.doc().unwrap().wb, &start)); + assert_eq!(undo_labels(&s), undo); + assert!(!s.in_group(s.doc().unwrap().uid)); + set(&mut s, "A1", "free"); + assert!(s.execute("macro.debug", json!({"action": "step"})).unwrap_err().to_string().contains("No macro is being debugged")); + // Stop Action on another workbook leaves the debugger alone. + s.execute("macro.debug", json!({"action": "start", "name": "Totals", "inputs": "B2:C5"})).unwrap(); + s.new_workbook(); + let r = s.execute("automate.stopAction", json!({})).unwrap(); + assert_eq!(r, json!({"stopped": false})); + assert!(s.macro_debug.is_some()); +} + +#[test] +fn the_debugger_runs_on_its_own_and_is_never_recorded() { + // As a step of a script, the debugger would outlive the script's rollback of its groups. + let mut s = s(); + stepper(&mut s); + let start = s.doc().unwrap().wb.clone(); + let commands = json!([ + {"command": "macro.debug", "params": {"action": "start", "name": "Walk", "inputs": "B2:C5"}}, + {"command": "cell.set", "params": {"cell": "A1", "input": "after"}} + ]); + s.execute("automate.newScript", json!({"name": "S", "commands": commands})).unwrap(); + let after_script = s.doc().unwrap().wb.clone(); + let e = s.execute("automate.runScript", json!({"name": "S"})).unwrap_err().to_string(); + assert!(e.contains("runs on its own"), "{e}"); assert!(s.macro_debug.is_none()); - s.execute("automate.stopAction", json!({})).unwrap(); + assert!(Arc::ptr_eq(&s.doc().unwrap().wb, &after_script)); + assert!(!s.in_group(s.doc().unwrap().uid)); + set(&mut s, "A1", "free"); + assert!(!Arc::ptr_eq(&s.doc().unwrap().wb, &start)); + // Neither recorder keeps it. + s.start_recording(); + s.execute("automate.recordActions", json!({})).unwrap(); + debug(&mut s, json!({"action": "start", "name": "Walk", "inputs": "B2:C5"})); + debug(&mut s, json!({"action": "stop"})); + let saved = s.execute("automate.recordActions", json!({"stop": true, "name": "R"})).unwrap(); + assert_eq!(saved["steps"], 0, "{saved}"); + let recorded = s.stop_recording().unwrap(); + assert!(!recorded.steps.iter().any(|st| st.id == "macro.debug"), "{:?}", recorded.steps); +} + +#[test] +fn a_debugger_whose_group_was_closed_elsewhere_locks_nothing() { + let mut s = s(); + stepper(&mut s); + let uid = s.doc().unwrap().uid; + debug(&mut s, json!({"action": "start", "name": "Walk", "inputs": "B2:C5"})); + debug(&mut s, json!({"action": "step"})); + // As if a command had rolled back the open groups without telling the debugger. + assert!(s.abort_groups(uid)); + set(&mut s, "A1", "free"); + assert!(s.macro_debug.is_none(), "the lock let the edit through and ended the debugger"); + // A debugger action says the run had ended; nothing is reported as kept or rolled back. + debug(&mut s, json!({"action": "start", "name": "Walk", "inputs": "B2:C5"})); + debug(&mut s, json!({"action": "step"})); + assert!(s.abort_groups(uid)); + let e = s.execute("macro.debug", json!({"action": "stop", "keep": true})).unwrap_err().to_string(); + assert!(e.contains("no longer being debugged"), "{e}"); + assert!(s.macro_debug.is_none()); + // Starting again works at once. + debug(&mut s, json!({"action": "start", "name": "Walk", "inputs": "B2:C5"})); + assert!(s.abort_groups(uid)); + assert_eq!(debug(&mut s, json!({"action": "start", "name": "Walk", "inputs": "B2:C5"}))["state"], "paused"); +} + +/// Whether `a` on sheet `sheet` is bold. +fn bold_on(s: &Session, sheet: &str, a: &str) -> bool { + let d = s.doc().unwrap(); + let sh = d.wb.sheet(d.wb.sheet_index(sheet).unwrap()).unwrap(); + d.wb.styles.get(sh.style_id(CellRef::parse(a).unwrap())).font.bold +} + +#[test] +fn between_steps_only_looking_around_runs_and_the_run_continues_where_it_was() { + let mut s = s(); + s.execute("home.insertSheet", json!({"name": "Other"})).unwrap(); + s.execute("sheet.activate", json!({"sheet": 0})).unwrap(); + put( + &mut s, + json!({"name": "Sel", "output": {"onConflict": "overwrite"}, "steps": [ + {"command": "selection.set", "params": {"range": "C3"}}, + {"command": "home.bold", "params": {}} + ]}), + ); + debug(&mut s, json!({"action": "start", "name": "Sel"})); + debug(&mut s, json!({"action": "step"})); + // Looking around runs: another sheet, another selection, Go To, Find, Copy, zoom. + s.execute("sheet.activate", json!({"sheet": "Other"})).unwrap(); + s.execute("selection.set", json!({"range": "A1"})).unwrap(); + s.execute("edit.goTo", json!({"reference": "B2"})).unwrap(); + s.execute("edit.copy", json!({})).unwrap(); + s.execute("view.zoom", json!({"percent": 150})).unwrap(); + assert!(s.execute("selection.stats", json!({})).is_ok(), "queries run"); + // Anything else waits, undoable or not, saving included. + let path = std::env::temp_dir().join(format!("gc-macro-debug-save-{}.xlsx", std::process::id())); + let busy = |r: Result| matches!(&r, Err(EngineError::Disabled(_, why)) if why == crate::macros::debug::BUSY); + assert!(busy(s.execute("view.customViews", json!({"save": "Mine"})))); + assert!(busy(s.execute("view.pageLayout", json!({})))); + assert!(busy(s.execute("file.saveAs", json!({"path": path.to_string_lossy()})))); + assert!(busy(s.execute("file.save", json!({})))); + assert!(busy(s.execute("automate.recordActions", json!({})))); + assert!(busy(s.execute("cell.set", json!({"cell": "A1", "input": "x"})))); + assert!(!path.exists(), "nothing was saved"); + // The next step runs on the run's sheet and selection, as macro.run would. + let st = debug(&mut s, json!({"action": "step"})); + assert_eq!(st["state"], "finished", "{st}"); + assert!(bold_on(&s, "Sheet1", "C3")); + assert!(!bold_on(&s, "Other", "B2") && !bold_on(&s, "Other", "A1")); + let d = s.doc().unwrap(); + assert_eq!((d.wb.active_sheet, d.selection.active), (0, CellRef::parse("C3").unwrap())); +} + +#[test] +fn a_panic_inside_a_step_is_that_steps_error_and_the_run_goes_on() { + crate::register_test_command("test.debugInnerPanics", |_, _| panic!("inner")); + // A step whose command catches a nested command's panic as an error and goes on editing. + crate::register_test_command("test.debugSwallowsPanic", |s, _| { + let _ = s.execute("test.debugInnerPanics", json!({})); + s.execute("cell.set", json!({"cell": "B1", "input": "after"})) + }); + let mut s = s(); + put( + &mut s, + json!({"name": "M", "output": {"onConflict": "overwrite"}, "steps": [ + {"command": "cell.set", "params": {"cell": "A1", "input": "x"}}, + {"command": "test.debugSwallowsPanic", "params": {}} + ]}), + ); + let undo = undo_labels(&s); + debug(&mut s, json!({"action": "start", "name": "M"})); + debug(&mut s, json!({"action": "step"})); + let st = debug(&mut s, json!({"action": "step"})); + assert_eq!(st["state"], "finished", "{st}"); + assert_eq!((v(&s, "A1").display(), v(&s, "B1").display()), ("x".into(), "after".into())); + let after = undo_labels(&s); + assert_eq!((after.len(), after.last().map(String::as_str)), (undo.len() + 1, Some("Run Macro: M")), "{after:?}"); + s.execute("edit.undo", json!({})).unwrap(); + assert!(v(&s, "A1").is_empty() && v(&s, "B1").is_empty()); +} + +#[test] +fn debugger_parameters_are_checked_before_anything_changes() { + let mut s = s(); + stepper(&mut s); + let start = |bps: Json| json!({"action": "start", "name": "Walk", "inputs": "B2:C5", "breakpoints": bps}); + let e = s.execute("macro.debug", start(json!(["9"]))).unwrap_err().to_string(); + assert!(e.contains("has no step 9 for a breakpoint"), "{e}"); + let many: Vec = (0..10_001).map(|_| "1".to_string()).collect(); + let e = s.execute("macro.debug", start(json!(many))).unwrap_err().to_string(); + assert!(e.contains("at most 10000"), "{e}"); + assert!(s.macro_debug.is_none() && !s.in_group(s.doc().unwrap().uid)); + debug(&mut s, start(json!([]))); + // `keep` is true or false: "true" would have read as false and rolled the run back. + assert!(s.execute("macro.debug", json!({"action": "stop", "keep": "true"})).is_err()); + assert!(s.execute("macro.debug", json!({"action": "step", "breakpointsOnly": "yes"})).is_err()); + assert!(s.execute("macro.debug", json!({"action": 5})).is_err()); + // An unknown action or step path changes nothing, breakpoints included. + assert!(s.execute("macro.debug", json!({"action": "leap", "breakpoints": ["2"]})).is_err()); + assert!(s.execute("macro.debug", json!({"action": "step", "breakpoints": ["2.x"]})).is_err()); + assert!(s.execute("macro.debug", json!({"action": "runTo", "step": "9", "breakpoints": ["2"]})).is_err()); + let st = debug(&mut s, json!({"action": "state"})); + assert_eq!((st["current"]["step"].as_str(), &st["breakpoints"]), (Some("1"), &json!([])), "{st}"); + // `breakpointsOnly` alone applies to the list the run has (none here): the flag on 3.1 no + // longer stops it. + let st = debug(&mut s, json!({"action": "continue", "breakpointsOnly": true})); + assert_eq!(st["state"], "finished", "{st}"); +} + +#[test] +fn the_debuggers_answers_count_its_own_steps_and_say_what_stays() { + // `executed` counts the run's steps, not the footprint's dry run before it. + let mut s = s(); + data(&mut s); + put(&mut s, totals()); + let st = debug(&mut s, json!({"action": "start", "name": "Totals", "inputs": "B2:C5"})); + assert_eq!(st["executed"], 0, "{st}"); + assert_eq!(debug(&mut s, json!({"action": "step"}))["executed"], 1); + let st = debug(&mut s, json!({"action": "continue"})); + assert_eq!((st["state"].as_str(), st["executed"].as_u64()), (Some("finished"), Some(3)), "{st}"); + // A shift at the start is reported, as macro.run reports it. + let mut s = super::s(); + data(&mut s); + set(&mut s, "E2", "old"); + put(&mut s, totals()); + let st = debug(&mut s, json!({"action": "start", "name": "Totals", "inputs": "B2:C5", "onConflict": "shiftDown"})); + assert_eq!(st["shifted"], "shiftDown", "{st}"); + assert_eq!(debug(&mut s, json!({"action": "stop"}))["shifted"], "shiftDown"); + // The files the run wrote stay when the workbook is put back: a failed step, a Stop step + // that rolls back, and stopping without keeping each say so. + let dir = std::env::temp_dir().join(format!("gc-macro-debug-files-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let export = json!({"command": "file.exportCsv", "params": {"path": format!("{}/out.csv", dir.to_string_lossy())}}); + for (last, how) in [ + (json!({"command": "cell.set", "params": {"cell": "Nowhere!A2", "input": "two"}}), "failed"), + (json!({"type": "stop", "message": "undo", "rollback": true}), "stopped"), + (json!({"command": "cell.set", "params": {"cell": "A2", "input": "two"}}), "stop"), + ] { + let mut s = super::s(); + put(&mut s, json!({"name": "F", "output": {"onConflict": "overwrite"}, "permissions": ["file.exportCsv"], "steps": [export, last]})); + debug(&mut s, json!({"action": "start", "name": "F", "approve": true})); + debug(&mut s, json!({"action": "step"})); + let st = if how == "stop" { debug(&mut s, json!({"action": "stop"})) } else { debug(&mut s, json!({"action": "step"})) }; + let msg = st["message"].as_str().unwrap_or_default(); + assert!(msg.contains("the file step 1 (file.exportCsv) wrote stays") && !msg.contains("Nothing was changed"), "{how}: {st}"); + assert_eq!(st["wroteFiles"], json!(["step 1 (file.exportCsv)"]), "{how}: {st}"); + } + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn the_debugger_lists_deleted_and_renamed_sheets_and_caps_the_list() { + let mut s = s(); + s.execute("home.insertSheet", json!({"name": "Sheet2"})).unwrap(); + s.execute("home.insertSheet", json!({"name": "Sheet3"})).unwrap(); + s.execute("sheet.activate", json!({"sheet": 0})).unwrap(); + put( + &mut s, + json!({"name": "Sheets", "output": {"onConflict": "overwrite"}, "steps": [ + {"command": "sheet.rename", "params": {"sheet": "Sheet2", "name": "Renamed"}}, + {"command": "home.deleteSheet", "params": {"sheet": "Sheet3"}}, + {"command": "cell.set", "params": {"cell": "A1", "input": "end"}} + ]}), + ); + debug(&mut s, json!({"action": "start", "name": "Sheets"})); + debug(&mut s, json!({"action": "step"})); + let st = debug(&mut s, json!({"action": "step"})); + let changed = st["changed"].as_array().unwrap(); + assert!(changed.contains(&json!("Renamed (renamed from Sheet2)")), "{st}"); + assert!(changed.contains(&json!("Sheet3 (deleted sheet)")), "{st}"); + assert!(!changed.iter().any(|c| c.as_str().is_some_and(|c| c.contains("(new sheet)"))), "{st}"); + // As if a step had written cells that make more ranges than the answer lists, on two sheets. + let d = s.doc_mut().unwrap(); + let mut wb = (*d.wb).clone(); + for sh in wb.sheets.iter_mut() { + let sh = Arc::make_mut(sh); + for r in 0..600u32 { + sh.set_value(CellRef::new(r + 10, r % 2), gridcraft_core::Value::Number(1.0)); + } + } + d.wb = Arc::new(wb); + let st = debug(&mut s, json!({"action": "state"})); + assert_eq!(st["changed"].as_array().unwrap().len(), 500, "{}", st["changed"].as_array().unwrap().len()); + assert_eq!(st["changedCells"], 1200); + debug(&mut s, json!({"action": "stop"})); +} + +#[test] +fn the_debugger_shows_a_repeat_over_whole_columns_as_the_run_goes_over_them() { + let mut s = s(); + data(&mut s); + put( + &mut s, + json!({"name": "Cols", "output": {"onConflict": "overwrite"}, "steps": [ + {"type": "repeat", "over": "B:C", "of": "columns", "body": [{"command": "cell.set", "params": {"cell": "Z1", "input": "x"}}]} + ]}), + ); + let st = debug(&mut s, json!({"action": "start", "name": "Cols"})); + assert_eq!(st["current"]["over"], "B2:C5", "{st}"); + debug(&mut s, json!({"action": "stop"})); } #[test] From f66ebd6fb20295cdf322833d1e6f0f16ed9a14b8 Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Sun, 25 Oct 2026 03:17:42 -0400 Subject: [PATCH 3/5] Docs: debugging macros: actions, start options, the lock and the time limit per action - The start options are those of `macro.run`, refused as it refuses them. - The time limit applies to each debugger action; the step and parameter limits to the whole run. - The lock lists what runs between steps, and says the sheet and selection are put back. - Stop Action ends the run; the debugger is never a step and never recorded. - The answers' fields, including `changedCells`, `shifted`, `wroteFiles` and `message`. --- docs/macros.md | 52 ++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 42 insertions(+), 10 deletions(-) diff --git a/docs/macros.md b/docs/macros.md index 4006096a7..5b9d9be00 100644 --- a/docs/macros.md +++ b/docs/macros.md @@ -62,7 +62,7 @@ The engine has no macro recorder or management commands in this version, and no pass through a Repeat counts), passes its commands at most 256 MB of parameters, all steps together, and takes at most 60 seconds, all three shared by the footprint dry runs and the run itself (`timeLimit` to change the time; the web build has no clock, so there only the step and - parameter limits apply). + parameter limits apply). The debugger gives each of its actions the time limit on its own. ## Tokens @@ -145,15 +145,47 @@ on, as after any recalculation. ## Debugging -`macro.debug {action, …}` with `start` (`name`, `inputs`, `breakpoints`), `step`, `continue`, -`runTo` (`step`: a path), `stop` (`keep: true` keeps the changes) and `state`. `breakpoints` can -be given with any action; `breakpointsOnly: true` makes the list replace the steps' own flags. The -run stays one undo step; while it is open, edits of the workbooks it touched (its own, and any -other its steps changed) are refused with "Finish or stop the macro first", also when another -command makes them (stopping Record Actions saves a script), and so is Automate › Stop Action, -which would roll the run's undo group back under the debugger; `stop` ends the run instead. Other -workbooks stay editable. Each answer gives the next step with its parameters as they would run, the parameters -of the step that just ran, and the cells changed so far. +`macro.debug {action, …}` runs a macro a step at a time. The actions are: + +- `start` with `name` and the options of `macro.run` (`inputs`, `onConflict`, `overwrite`, + `approve`, `timeLimit`), refused as `macro.run` refuses them when they are of the wrong kind. The + checks, the approval and the footprint come first, as for `macro.run`; then the run pauses + before its first step. +- `step` runs the next step; `continue` runs to the next breakpoint or the end; `runTo` (`step`: + a path) runs to that step, stopping at a breakpoint on the way. +- `stop` ends the run: `keep: true` keeps the changes as one undo step, otherwise they are rolled + back. +- `state` answers without running anything (the default while a macro is being debugged). + +`breakpoints` (a list of step paths of the macro, at most 10,000) can be given with any action and +replaces the list set before; `breakpointsOnly: true` makes the list replace the steps' own +breakpoint flags, also when it is given without a list. An unknown action, a breakpoint or `runTo` +path the macro doesn't have, and `keep` or `breakpointsOnly` that isn't `true` or `false` are +refused before anything changes. The time limit applies to each action on its own (`start` with +its dry runs, and each `step`, `continue` or `runTo`); the step and parameter limits apply to the +whole run, counting the dry runs. + +Each answer gives the next step with its parameters as they would run (a Repeat over whole rows or +columns shows the part of the sheet in use, as the run goes over it), the parameters of the step +that just ran, the steps the run has executed (the dry runs not counted), and what the run has +changed in its workbook: `changed` lists at most 500 entries (cell ranges, and sheets added, +deleted or renamed) and `changedCells` counts the changed cells on the sheets it had at the start. `shifted` names the rule that made +room at the start, and `wroteFiles` the files the run wrote, which stay when the run is rolled +back; the answer to a rolled-back run says so in `message`. A run that ends (its last step, a +Stop step, or a failed step, which rolls it back) or is stopped answers with `state` `finished`, +`stopped` or `failed`. + +While a macro is being debugged, its workbook is locked between steps: only `macro.debug`, queries +(commands that only read), commands that look around (`selection.*`, Select All, `sheet.activate`, +`sheet.next` and `sheet.previous`, Go To, Go To Special, Find, Copy and the zoom commands), Switch +Window, Close (which ends the run with its workbook) and Automate › Stop Action run there. +Everything else, undoable or not, saving included, is refused with "Finish or stop the macro +first", also when another command runs it (stopping Record Actions saves a script). Other +workbooks stay editable. The person may look around between steps: before the next step runs, the +debugger puts the active sheet and the selection back where the run left them, so the run goes on +as `macro.run` would run it. A zoom change made meanwhile is part of the run, and is undone with +it. Stop Action rolls the run back and ends it, and its answer names the macro. The debugger runs +on its own, never as a step of a script or macro, and is never recorded. ## Storage From 198fe482a1d76d9349be88d4e748c3cbca0fe308 Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Sun, 25 Oct 2026 03:19:59 -0400 Subject: [PATCH 4/5] Engine: the run's check of each step finds the inputs on their sheet after a sheet moved - The check for writes over data compared the inputs' sheet with the sheet's index when the run started, while the inputs follow their cells: after a step inserted a sheet in front, a step writing into its own input on the moved sheet was stopped as writing over data. It now uses the sheet's index now. Test: the input case in `inputs_and_the_output_follow_their_cells_when_a_step_moves_them`. - Docs: the page says `macro.run` checks the macro, its inputs and parameters and the policy before it runs, and that a formula whose references a move rewrites counts as changed too. --- crates/engine/src/macros/run.rs | 3 ++- crates/engine/src/tests/macros.rs | 13 ++++++++++++- docs/macros.md | 10 ++++++---- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/macros/run.rs b/crates/engine/src/macros/run.rs index 3d293e609..917deed74 100644 --- a/crates/engine/src/macros/run.rs +++ b/crates/engine/src/macros/run.rs @@ -508,7 +508,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) diff --git a/crates/engine/src/tests/macros.rs b/crates/engine/src/tests/macros.rs index 0df51122c..a4871f2f2 100644 --- a/crates/engine/src/tests/macros.rs +++ b/crates/engine/src/tests/macros.rs @@ -191,10 +191,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"); diff --git a/docs/macros.md b/docs/macros.md index c0a09890f..d4f1cc43e 100644 --- a/docs/macros.md +++ b/docs/macros.md @@ -9,9 +9,10 @@ error (a file it already wrote stays), stops before it writes over data it may n touch files without permission. Agents run macros with `macro.run` like any other command. -This page describes the macro format, its tokens, and running macros. The engine has no check of a -macro before it runs, no macro debugger, recorder or management commands in this version, and no -macro interface. +This page describes the macro format, its tokens, and running macros. `macro.run` checks the +macro, its inputs, its parameters and the policy before it runs, and each step as it runs; this +version has no command that checks a macro without running it, no macro debugger, recorder or +management commands, and no macro interface. ## A macro @@ -115,7 +116,8 @@ refused rather than read as the header. step that writes over cells outside the inputs that held data when the run started, or deletes sheets or objects that were there, stops the run, which rolls the workbook back (a file already written stays). Moving such cells, as inserting or deleting rows or cells above them does, - counts as writing over them: the check compares cells by place. The rule is `onConflict` (or + counts as writing over them, and so does a formula whose references such a move rewrites: the + check compares cells by place, so it may ask about data that only moved, never the other way. The rule is `onConflict` (or the macro's `output.onConflict`): `ask` (the call fails with `confirm: …`; `overwrite: true` answers it), `fail` or `overwrite`. Shifting the data to make room (`shiftDown`, `shiftRight`) needs to know where the macro writes before it runs, which this version doesn't From dadffdb33f2b9fc212cf4b55996c9a38815405e5 Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Sun, 25 Oct 2026 03:21:15 -0400 Subject: [PATCH 5/5] Engine: macro.check shifts and checks again, as the run does, before it says ok Under `shiftDown` or `shiftRight` with conflicts, `macro.check` looked only for the refusals made before a shift, so it said `ok` for a run the second footprint stops (an anchor that moves with the shifted data). It now shifts the data inside an undo group, takes the footprint again, rolls both back and puts the session back, and reports what still stands in the way. - Test: the `Named` case and a shift that makes room in `shifting_makes_room_and_is_checked_again`. --- crates/engine/src/macros/run.rs | 26 +++++++++++++++++++++++++- crates/engine/src/tests/macros.rs | 13 +++++++++++++ docs/macros.md | 3 ++- 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/macros/run.rs b/crates/engine/src/macros/run.rs index bff4e0b90..6a6d3f5c8 100644 --- a/crates/engine/src/macros/run.rs +++ b/crates/engine/src/macros/run.rs @@ -1488,6 +1488,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()?; @@ -1549,7 +1570,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 57307ee8c..50fbcfc14 100644 --- a/crates/engine/src/tests/macros.rs +++ b/crates/engine/src/tests/macros.rs @@ -304,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 fe169fb3b..4286fb487 100644 --- a/docs/macros.md +++ b/docs/macros.md @@ -143,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.