From 3f5f6ea4f1fc9f1ab6bb66d45f85d2693cd16b8e Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Wed, 21 Oct 2026 16:59:29 -0400 Subject: [PATCH] Engine: a name given when a recording stops never replaces an existing macro Stopping a recording with a name that a macro already has (in any case) saved over that macro's steps, inputs and description, with only Undo to get them back. A taken name now gets a suffix ("Tidy (2)"), and the save passes replace: false as a backstop. --- crates/engine/src/cmd/macros.rs | 7 ++++--- crates/engine/src/tests/macros.rs | 15 +++++++++++++++ docs/macros.md | 3 ++- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/macros.rs b/crates/engine/src/cmd/macros.rs index 383471f53..cd1f36c67 100644 --- a/crates/engine/src/cmd/macros.rs +++ b/crates/engine/src/cmd/macros.rs @@ -45,7 +45,7 @@ pub fn specs() -> Vec { "Record Macro", ["View", "Macros"], None, - "{stop?: bool, relative?: bool (default true; when starting), name?, description? (when stopping)} 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, 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 ), @@ -217,8 +217,9 @@ fn save_recording(s: &mut Session, p: &Json, rec: &crate::record::Recorder, star relative: true, }); let wb = d.wb.clone(); + // A name another macro has gets a suffix, so stopping never replaces a macro. let name = match str_param(p, "name").map(str::trim).filter(|n| !n.is_empty()) { - Some(n) => n.to_string(), + Some(n) => crate::macros::unique_name(&wb, n), None => crate::macros::next_name(&wb), }; let mut converted = crate::macros::recorder::convert(&name, &start, &rec.steps, &wb); @@ -231,7 +232,7 @@ fn save_recording(s: &mut Session, p: &Json, rec: &crate::record::Recorder, star // Saved into that workbook even when another one is active now. let active = s.active_index(); s.set_active(at); - let saved = s.execute("macro.save", json!({"macro": crate::macros::macro_json(&converted.mac)})); + let saved = s.execute("macro.save", json!({"macro": crate::macros::macro_json(&converted.mac), "replace": false})); s.set_active(active); saved?; Ok(json!({"recording": false, "saved": name, "steps": steps, "dropped": rec.dropped, "skipped": converted.skipped, "inputs": inputs})) diff --git a/crates/engine/src/tests/macros.rs b/crates/engine/src/tests/macros.rs index 323b64ed4..0339ad5ee 100644 --- a/crates/engine/src/tests/macros.rs +++ b/crates/engine/src/tests/macros.rs @@ -1126,3 +1126,18 @@ fn a_recording_is_saved_in_the_workbook_where_it_started() { assert!(!s.is_recording()); assert!(s.doc().unwrap().wb.macros.is_empty()); } + +#[test] +fn a_name_given_when_recording_stops_never_replaces_a_macro() { + let mut s = s(); + let original = json!({"command": "cell.set", "params": {"cell": "Z9", "input": "mine"}}); + put(&mut s, json!({"name": "Tidy", "description": "keep me", "steps": [original]})); + s.execute("macro.record", json!({"relative": false})).unwrap(); + set(&mut s, "A1", "recorded"); + let r = s.execute("macro.record", json!({"stop": true, "name": "tidy"})).unwrap(); + assert_eq!(r["saved"], "tidy (2)"); + let kept = crate::macros::parse_macro(&s.execute("macro.get", json!({"name": "Tidy"})).unwrap()).unwrap(); + assert_eq!((kept.name.as_str(), kept.description.as_str(), kept.steps.len()), ("Tidy", "keep me", 1)); + assert_eq!(recorded(&mut s, "Tidy"), ["cell.set {\"cell\":\"Z9\",\"input\":\"mine\"}"]); + assert_eq!(recorded(&mut s, "tidy (2)"), ["cell.set {\"cell\":\"A1\",\"input\":\"recorded\"}"]); +} diff --git a/docs/macros.md b/docs/macros.md index 235d1f2b4..9f1461fc2 100644 --- a/docs/macros.md +++ b/docs/macros.md @@ -119,7 +119,8 @@ of the step that just ran, and the cells changed so far. ## Recording `macro.record` toggles recording (`relative: false` for absolute references; `name` and -`description` when stopping). The macro is saved in the workbook where recording started, even if +`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 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), recording goes on, so stopping again keeps what was recorded. With relative references the