Engine: the macro recorder keeps a step's own sheet, stays bounded, and can be cancelled

Six fixes from a review of the recorder, each with a regression test (four
fail against the old recorder; the other two bound work that grew with the
input):

- A step that names another sheet with its sheet parameter (cell.set and
  about fifty other commands) was tokenized as if on the input's sheet, so a
  recorded cell.set {sheet: Two, cell: C4} ran on Two at the input's
  relative place. Such a step keeps its references, like a sheet-qualified
  one.
- Inlining recorded runs of other macros and scripts stops at the macro's
  step limit and counts the rest in dropped, instead of expanding every run
  first (a thousand runs of a long macro made millions of steps).
- macro.record {cancel: true} ends a recording without saving it: the way
  out of a recording whose macro is too large for the workbook, which could
  neither be saved nor ended.
- Formula tokenization reads each reference in the formula and matches it,
  instead of listing every column and row of the example (an example as
  large as a sheet made a million candidates per formula).
- A selection of several areas keeps its active cell as it is; a token for
  the active cell alone put it outside the areas on playback.
- Steps recorded in another workbook are counted in dropped.

crates/engine/src/cmd/macros.rs

@@ -45,7 +45,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Record Macro",
             ["View", "Macros"],
             None,
-            "{stop?: bool, relative?: bool (default true; when starting), name?, description? (when stopping; a taken name gets a suffix)} toggles recording; with relative references the selection when it starts is the example input → {recording, relative?, input?} or {recording: false, saved, steps, dropped, skipped, inputs}",
+            "{stop?: bool, cancel?: bool (ends a recording without saving it), relative?: bool (default true; when starting), name?, description? (when stopping; a taken name gets a suffix)} toggles recording; with relative references the selection when it starts is the example input → {recording, relative?, input?} or {recording: false, saved, steps, dropped, skipped, inputs}",
             has_doc,
             record
         ),
@@ -163,6 +163,13 @@ fn check(s: &mut Session, p: &Json) -> Result<Json> {
 }
 
 fn record(s: &mut Session, p: &Json) -> Result<Json> {
+    // `cancel` ends a recording without saving it: the way out of a recording whose macro
+    // cannot be saved (too large for the workbook, for one).
+    if bool_param(p, "cancel").unwrap_or(false) {
+        let was = s.stop_recording().is_some();
+        s.macro_recording = None;
+        return Ok(json!({"recording": false, "cancelled": was}));
+    }
     if bool_param(p, "stop").unwrap_or(false) || s.is_recording() {
         return stop_recording(s, p);
     }
@@ -228,8 +235,9 @@ fn save_recording(s: &mut Session, p: &Json, rec: &crate::record::Recorder, star
         None => crate::macros::next_name(&wb),
     };
     let mut converted = crate::macros::recorder::convert(&name, &start, &rec.steps, &wb);
-    // Steps past the macro limit are left out and counted with the ones the recorder dropped.
-    let cut = converted.mac.steps.len().saturating_sub(gridcraft_model::MAX_MACRO_STEPS);
+    // Steps past the macro limit, and ones recorded in another workbook, are left out and counted
+    // with the ones the recorder dropped.
+    let cut = converted.cut.saturating_add(converted.mac.steps.len().saturating_sub(gridcraft_model::MAX_MACRO_STEPS));
     converted.mac.steps.truncate(gridcraft_model::MAX_MACRO_STEPS);
     if let Some(desc) = str_param(p, "description") {
         converted.mac.description = desc.chars().take(gridcraft_model::MAX_MACRO_TEXT).collect();

crates/engine/src/macros/recorder.rs

@@ -15,7 +15,7 @@
 //!   shape, header and column types are inferred from it.
 
 use gridcraft_core::{CellRef, RangeRef, Value};
-use gridcraft_model::{ColType, InputKind, Macro, NamedInput, Step, StepKind, Workbook};
+use gridcraft_model::{ColType, InputKind, MAX_MACRO_STEPS, Macro, NamedInput, Step, StepKind, Workbook};
 use serde_json::{Map, Value as Json, json};
 
 use super::policy::{Policy, macro_policy};
@@ -41,15 +41,28 @@ pub struct Converted {
     pub mac: Macro,
     /// Commands left out (ids), in order.
     pub skipped: Vec<String>,
+    /// Steps not kept: past the macro's step limit (a recorded run of a long macro adds all of
+    /// its steps) or recorded in another workbook.
+    pub cut: usize,
 }
 
 /// Builds a macro called `name` from the recorded steps.
 pub fn convert(name: &str, start: &RecordingStart, steps: &[RecordedStep], wb: &Workbook) -> Converted {
     let mut skipped = Vec::new();
     // Commands as they'll be played back: (id, params, sheet before, selection before, whether
-    // they came from another macro and stay as written).
+    // they came from another macro and stay as written). At most the macro's step limit are
+    // gathered: a recording of a thousand runs of a long macro would otherwise expand to
+    // millions of steps before the limit cut them back.
     let mut cmds: Vec<(String, Json, usize, String, Selection, bool)> = Vec::new();
+    let mut cut = steps.iter().filter(|st| st.doc != start.doc).count();
     let refused = |id: &str, params: &Json| matches!(macro_policy(id, params, crate::Trust::Local), Policy::Deny(_) | Policy::NeedsPermission(_));
+    let mut push = |cmds: &mut Vec<(String, Json, usize, String, Selection, bool)>, cmd: (String, Json, usize, String, Selection, bool)| {
+        if cmds.len() < MAX_MACRO_STEPS {
+            cmds.push(cmd);
+        } else {
+            cut = cut.saturating_add(1);
+        }
+    };
     for st in steps.iter().filter(|st| st.doc == start.doc) {
         let id = st.id.as_str();
         if id == "macro.run" {
@@ -64,7 +77,7 @@ pub fn convert(name: &str, start: &RecordingStart, steps: &[RecordedStep], wb: &
                             skipped.push(cid.clone());
                             continue;
                         }
-                        cmds.push((cid.clone(), params.clone(), st.sheet, st.sheet_name.clone(), st.selection.clone(), true));
+                        push(&mut cmds, (cid.clone(), params.clone(), st.sheet, st.sheet_name.clone(), st.selection.clone(), true));
                     }
                 }
                 _ => skipped.push(id.to_string()),
@@ -80,7 +93,7 @@ pub fn convert(name: &str, start: &RecordingStart, steps: &[RecordedStep], wb: &
                             skipped.push(cid);
                             continue;
                         }
-                        cmds.push((cid, params, st.sheet, st.sheet_name.clone(), st.selection.clone(), true));
+                        push(&mut cmds, (cid, params, st.sheet, st.sheet_name.clone(), st.selection.clone(), true));
                     }
                 }
                 None => skipped.push(id.to_string()),
@@ -91,7 +104,7 @@ pub fn convert(name: &str, start: &RecordingStart, steps: &[RecordedStep], wb: &
             skipped.push(id.to_string());
             continue;
         }
-        cmds.push((id.to_string(), st.params.clone(), st.sheet, st.sheet_name.clone(), st.selection.clone(), false));
+        push(&mut cmds, (id.to_string(), st.params.clone(), st.sheet, st.sheet_name.clone(), st.selection.clone(), false));
     }
 
     // Sheet and selection changes made without a command: where a command ran on another sheet,
@@ -131,7 +144,14 @@ pub fn convert(name: &str, start: &RecordingStart, steps: &[RecordedStep], wb: &
     let steps: Vec<Step> = collapsed
         .into_iter()
         .map(|(id, params, on, kept)| {
-            let here = on.eq_ignore_ascii_case(&start.sheet_name);
+            // A step that names another sheet with its `sheet` parameter runs there, whatever
+            // sheet was active: its unqualified references are that sheet's cells.
+            let names_other_sheet = params.get("sheet").is_some_and(|v| match v {
+                Json::String(n) => !n.eq_ignore_ascii_case(&start.sheet_name),
+                Json::Number(i) => i.as_u64() != Some(start.sheet as u64),
+                _ => false,
+            });
+            let here = on.eq_ignore_ascii_case(&start.sheet_name) && !names_other_sheet;
             let params = if start.relative && !kept { relative(&params, example, &start.sheet_name, here, &mut used_input, None) } else { params };
             Step::command(id, params)
         })
@@ -140,7 +160,7 @@ pub fn convert(name: &str, start: &RecordingStart, steps: &[RecordedStep], wb: &
     if start.relative && used_input {
         mac.inputs = vec![infer_input(wb, start.sheet, example)];
     }
-    Converted { mac, skipped }
+    Converted { mac, skipped, cut }
 }
 
 fn selection_params(sel: &Selection) -> Json {
@@ -153,22 +173,28 @@ fn selection_params(sel: &Selection) -> Json {
 fn relative(v: &Json, example: RangeRef, sheet: &str, here: bool, used: &mut bool, key: Option<&str>) -> Json {
     match v {
         Json::Object(o) => {
+            // A selection of several areas stays as it is, its active cell with it: a token for
+            // the active cell alone would put it outside the areas on playback.
+            let several_areas = o.get("range").and_then(Json::as_str).is_some_and(|r| r.contains(','));
             let mut out = Map::new();
             for (k, x) in o {
-                out.insert(k.clone(), relative(x, example, sheet, here, used, Some(k)));
+                let x = if several_areas && k == "active" { x.clone() } else { relative(x, example, sheet, here, used, Some(k)) };
+                out.insert(k.clone(), x);
             }
             Json::Object(out)
         }
         Json::Array(a) => Json::Array(a.iter().map(|x| relative(x, example, sheet, here, used, key)).collect()),
         Json::String(t) if key.is_some_and(super::resolve::is_ref_key) => {
-            // A list of areas ("A1:B2,D4") stays as it is unless every part has a token.
-            let parts: Option<Vec<String>> = t.split(',').map(|part| ref_token(part, example, sheet, here)).collect();
-            match parts {
-                Some(p) if p.len() == 1 => {
+            // One area becomes a token; a list of areas ("A1:B2,D4") stays as it is.
+            if t.contains(',') {
+                return v.clone();
+            }
+            match ref_token(t, example, sheet, here) {
+                Some(token) => {
                     *used = true;
-                    json!(p.join(""))
+                    json!(token)
                 }
-                _ => v.clone(),
+                None => v.clone(),
             }
         }
         Json::String(t) if here && t.starts_with('=') && example.count() > 1 => {
@@ -255,22 +281,9 @@ fn token(t: RangeRef, r: RangeRef) -> Option<String> {
 /// Replaces references in a formula that are exactly the example input, one of its columns or
 /// rows, or a column without its first row, with `{{token}}`.
 fn formula_tokens(f: &str, r: RangeRef) -> (String, bool) {
-    let mut candidates: Vec<(String, String)> = vec![(r.a1(), "Input".into())];
-    for (i, col) in (r.start.col..=r.end.col).enumerate() {
-        let n = i + 1;
-        candidates.push((RangeRef::new(CellRef::new(r.start.row, col), CellRef::new(r.end.row, col)).a1(), format!("Input.col({n})")));
-        if r.height() > 2 {
-            candidates
-                .push((RangeRef::new(CellRef::new(r.start.row + 1, col), CellRef::new(r.end.row, col)).a1(), format!("Input.rows(2..).col({n})")));
-        }
-    }
-    if r.width() > 1 {
-        for (i, row) in (r.start.row..=r.end.row).enumerate() {
-            candidates.push((RangeRef::new(CellRef::new(row, r.start.col), CellRef::new(row, r.end.col)).a1(), format!("Input.row({})", i + 1)));
-        }
-    }
-    // Longest first, so `B2:B10` isn't taken for a `B2` cell reference.
-    candidates.sort_by_key(|(a1, _)| std::cmp::Reverse(a1.len()));
+    // Each reference in the formula text is read and matched on its own, so the work is the
+    // formula's length whatever the example's size (an example of a whole sheet has a million
+    // rows, which no list of candidates may grow with).
     let mut out = String::with_capacity(f.len());
     let mut changed = false;
     let (mut i, mut in_text) = (0, false);
@@ -282,19 +295,20 @@ fn formula_tokens(f: &str, r: RangeRef) -> (String, bool) {
             in_text = !in_text;
         }
         let prev_ok = i == 0 || !bytes.get(i - 1).is_some_and(|b| b.is_ascii_alphanumeric() || matches!(b, b'$' | b'!' | b'_' | b'.' | b':'));
-        if !in_text && prev_ok {
-            let hit = candidates.iter().find(|(a1, _)| {
-                rest.starts_with(a1.as_str())
-                    && !rest
-                        .get(a1.len()..)
-                        .and_then(|x| x.chars().next())
-                        .is_some_and(|c| c.is_ascii_alphanumeric() || matches!(c, ':' | '!' | '(' | '_'))
-            });
-            if let Some((a1, tok)) = hit {
+        if !in_text && prev_ok && ch.is_ascii_alphabetic() {
+            // The longest run of reference characters here: letters, digits and one `:`.
+            let n = rest.bytes().take_while(|b| b.is_ascii_alphanumeric() || *b == b':').count();
+            let text = rest.get(..n).unwrap_or("");
+            let followed = rest.get(n..).and_then(|x| x.chars().next()).is_some_and(|c| matches!(c, '!' | '(' | '_' | '$'));
+            if !followed
+                && !text.contains('$')
+                && let Some(t) = RangeRef::parse(text)
+                && let Some(tok) = formula_token(t, r)
+            {
                 out.push_str("{{");
-                out.push_str(tok);
+                out.push_str(&tok);
                 out.push_str("}}");
-                i += a1.len();
+                i += n;
                 changed = true;
                 continue;
             }
@@ -305,6 +319,29 @@ fn formula_tokens(f: &str, r: RangeRef) -> (String, bool) {
     (out, changed)
 }
 
+/// The token for a reference inside a formula: exactly the example input, one of its columns or
+/// rows, or a column without its first row. Other references stay as they are.
+fn formula_token(t: RangeRef, r: RangeRef) -> Option<String> {
+    if t == r {
+        return Some("Input".into());
+    }
+    if !r.contains_range(&t) {
+        return None;
+    }
+    let col = usize::try_from(t.start.col.checked_sub(r.start.col)?).ok()?.checked_add(1)?;
+    let row = usize::try_from(t.start.row.checked_sub(r.start.row)?).ok()?.checked_add(1)?;
+    if t.start.col == t.end.col && t.start.row == r.start.row && t.end.row == r.end.row {
+        return Some(format!("Input.col({col})"));
+    }
+    if t.start.col == t.end.col && r.height() > 2 && t.start.row == r.start.row.checked_add(1)? && t.end.row == r.end.row {
+        return Some(format!("Input.rows(2..).col({col})"));
+    }
+    if r.width() > 1 && t.start.row == t.end.row && t.start.col == r.start.col && t.end.col == r.end.col {
+        return Some(format!("Input.row({row})"));
+    }
+    None
+}
+
 /// The example input's shape: a header when the first row is all text over other kinds of
 /// values, and each column's type when all its values agree.
 fn infer_input(wb: &Workbook, sheet: usize, r: RangeRef) -> NamedInput {

crates/engine/src/tests/macros.rs

@@ -1046,6 +1046,104 @@ fn a_relative_recording_keeps_references_on_another_sheet() {
     assert_eq!((v(&s, "C4").display(), v(&s, "D2").display()), ("x".into(), "12".into()));
 }
 
+#[test]
+fn a_step_that_names_another_sheet_keeps_its_references() {
+    // `sheet` names where the step runs whatever sheet is active, so its C4 is that sheet's C4.
+    let mut s = s();
+    s.execute("home.insertSheet", json!({"name": "Two"})).unwrap();
+    s.execute("sheet.activate", json!({"sheet": "Sheet1"})).unwrap();
+    data(&mut s);
+    s.execute("selection.set", json!({"range": "B2:C5"})).unwrap();
+    s.execute("macro.record", json!({})).unwrap();
+    s.execute("cell.set", json!({"sheet": "Two", "cell": "C4", "input": "x"})).unwrap();
+    s.execute("cell.set", json!({"sheet": "Sheet1", "cell": "C4", "input": "90"})).unwrap();
+    s.execute("macro.record", json!({"stop": true, "name": "Named"})).unwrap();
+    let steps = recorded(&mut s, "Named");
+    assert!(steps.contains(&"cell.set {\"cell\":\"C4\",\"input\":\"x\",\"sheet\":\"Two\"}".to_string()), "{steps:?}");
+    assert!(steps.contains(&"cell.set {\"cell\":\"Input.cell(3,2)\",\"input\":\"90\",\"sheet\":\"Sheet1\"}".to_string()), "{steps:?}");
+}
+
+#[test]
+fn inlining_recorded_runs_stops_at_the_step_limit() {
+    // A recorded run of a long macro adds all of its steps; the conversion gathers at most the
+    // macro's step limit and counts the rest, instead of expanding every run first.
+    let mut s = s();
+    let steps: Vec<Json> = (0..6_000).map(|_| json!({"command": "cell.set", "params": {"cell": "A1", "input": "x"}})).collect();
+    put(&mut s, json!({"name": "Big", "output": {"onConflict": "overwrite"}, "steps": steps}));
+    s.execute("macro.record", json!({})).unwrap();
+    s.execute("macro.run", json!({"name": "Big"})).unwrap();
+    s.execute("macro.run", json!({"name": "Big"})).unwrap();
+    let r = s.execute("macro.record", json!({"stop": true, "name": "Twice"})).unwrap();
+    assert_eq!((r["steps"].as_u64(), r["dropped"].as_u64()), (Some(10_000), Some(2_000)), "{r}");
+}
+
+#[test]
+fn a_recording_can_be_cancelled() {
+    let mut s = s();
+    data(&mut s);
+    let before = s.execute("macro.list", json!({})).unwrap();
+    s.execute("macro.record", json!({})).unwrap();
+    set(&mut s, "E2", "x");
+    let r = s.execute("macro.record", json!({"cancel": true})).unwrap();
+    assert_eq!(r, json!({"recording": false, "cancelled": true}));
+    assert!(!s.is_recording() && s.macro_recording.is_none());
+    assert_eq!(s.execute("macro.list", json!({})).unwrap(), before, "nothing was saved");
+    assert_eq!(s.execute("macro.record", json!({"cancel": true})).unwrap()["cancelled"], false);
+    // Recording works again afterwards.
+    s.execute("macro.record", json!({})).unwrap();
+    assert!(s.is_recording());
+    s.execute("macro.record", json!({"cancel": true})).unwrap();
+}
+
+#[test]
+fn formulas_are_tokenized_by_reading_their_references() {
+    // An example as large as a sheet has a million rows: each reference in the formula is read
+    // and matched on its own, instead of a list of every column and row of the example.
+    let mut s = s();
+    s.execute("selection.set", json!({"range": "A1:Z1000000"})).unwrap();
+    s.execute("macro.record", json!({})).unwrap();
+    s.execute("cell.set", json!({"cell": "AB1", "input": "=SUM(A1:A1000000)+SUM(A1:A3)+B2"})).unwrap();
+    s.execute("cell.set", json!({"cell": "AB2", "input": "=SUM(A1:Z1000000)&\"A1:A1000000\""})).unwrap();
+    s.execute("macro.record", json!({"stop": true, "name": "Wide"})).unwrap();
+    let steps = recorded(&mut s, "Wide");
+    // A single cell inside a formula stays as it is: only the input, its columns and its rows
+    // become tokens there.
+    assert!(steps.iter().any(|st| st.contains("=SUM({{Input.col(1)}})+SUM(A1:A3)+B2")), "{steps:?}");
+    assert!(steps.iter().any(|st| st.contains("=SUM({{Input}})&\\\"A1:A1000000\\\"")), "{steps:?}");
+}
+
+#[test]
+fn a_selection_of_several_areas_keeps_its_active_cell_as_it_is() {
+    let mut s = s();
+    data(&mut s);
+    s.execute("selection.set", json!({"range": "B2:C5"})).unwrap();
+    s.execute("macro.record", json!({})).unwrap();
+    // Several areas picked with the mouse: no command, so the recording adds a selection.set
+    // with the active cell before the next step.
+    let (a, b) = (gridcraft_core::RangeRef::parse("B2:C5").unwrap(), gridcraft_core::RangeRef::parse("E2:E5").unwrap());
+    s.doc_mut().unwrap().selection = crate::Selection { active: b.start, anchor: b.start, ranges: vec![a, b] };
+    s.execute("home.bold", json!({})).unwrap();
+    s.execute("macro.record", json!({"stop": true, "name": "Areas"})).unwrap();
+    let steps = recorded(&mut s, "Areas");
+    assert!(steps.contains(&"selection.set {\"active\":\"E2\",\"range\":\"B2:C5,E2:E5\"}".to_string()), "{steps:?}");
+    assert!(steps.iter().all(|st| !st.contains("\"active\":\"Input")), "{steps:?}");
+}
+
+#[test]
+fn steps_recorded_in_another_workbook_are_counted() {
+    let mut s = s();
+    data(&mut s);
+    let home = s.active_index();
+    s.execute("macro.record", json!({})).unwrap();
+    s.new_workbook();
+    set(&mut s, "A1", "elsewhere");
+    set(&mut s, "A2", "too");
+    s.set_active(home);
+    set(&mut s, "E2", "here");
+    let r = s.execute("macro.record", json!({"stop": true, "name": "Home"})).unwrap();
+    assert_eq!((r["steps"].as_u64(), r["dropped"].as_u64()), (Some(1), Some(2)), "{r}");
+}
+
 #[test]
 fn a_recorded_macro_run_is_inlined_only_when_its_steps_stand_alone() {
     let mut s = s();

docs/macros.md

@@ -120,7 +120,8 @@ of the step that just ran, and the cells changed so far.
 
 `macro.record` toggles recording (`relative: false` for absolute references; `name` and
 `description` when stopping; a name that another macro has gets a suffix, such as `Tidy (2)`, so
-stopping never replaces a macro). The macro is saved in the workbook where recording started, even if
+stopping never replaces a macro; `cancel: true` ends a recording without saving it, the way out of
+one whose macro is too large for the workbook). The macro is saved in the workbook where recording started, even if
 another one is active; if that workbook was closed, stopping ends the recording without saving it.
 When saving the recording fails (a name the check refuses, a workbook at its macro limit, a
 macro being debugged in that workbook), recording goes on, so stopping again keeps what was