From c442da92212b76ea15efd1e427954c6c3da0a478 Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Sat, 24 Oct 2026 02:29:00 -0400 Subject: [PATCH] Engine: stopping a recording says when it left the chosen shortcut off Record Macro checks the Ctrl+Shift letter when recording starts, but another macro can take it before recording stops; the save then leaves the shortcut off, and nothing said so. The result now names it in shortcutDropped, the toast after Stop says the macro was saved without it, and the macro page describes it. --- crates/engine/src/cmd/macros.rs | 24 +++++++++++++++--------- crates/engine/src/tests/macros.rs | 19 +++++++++++++++++++ crates/ui-egui/src/macro_ui.rs | 6 +++++- docs/macros.md | 26 +++++++++++++------------- 4 files changed, 52 insertions(+), 23 deletions(-) diff --git a/crates/engine/src/cmd/macros.rs b/crates/engine/src/cmd/macros.rs index de1edd367..f75cee6ef 100644 --- a/crates/engine/src/cmd/macros.rs +++ b/crates/engine/src/cmd/macros.rs @@ -46,7 +46,7 @@ pub fn specs() -> Vec { "Record Macro", ["View", "Macros"], None, - "{stop?: bool, cancel?: bool (ends a recording without saving it), relative?: bool (default true; when starting), name?, shortcut?: \"Ctrl+Shift+K\" | \"K\", description?} toggles recording; when starting, the name (new and valid) and the shortcut (free) are checked and kept for the save, and without a name the macro gets the next MacroN; when stopping, a name or description given then wins (a taken name gets a suffix); with relative references the selection when it starts is the example input → {recording, name?, 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?, shortcut?: \"Ctrl+Shift+K\" | \"K\", description?} toggles recording; when starting, the name (new and valid) and the shortcut (free) are checked and kept for the save, and without a name the macro gets the next MacroN; when stopping, a name or description given then wins (a taken name gets a suffix); with relative references the selection when it starts is the example input → {recording, name?, relative?, input?} or {recording: false, saved, steps, dropped, skipped, inputs, shortcutDropped? (the shortcut another macro took while recording)}", has_doc, record ), @@ -338,11 +338,15 @@ fn save_recording(s: &mut Session, p: &Json, rec: &crate::record::Recorder, star converted.mac.description = desc.chars().take(gridcraft_model::MAX_MACRO_TEXT).collect(); } cut = cut.saturating_add(crate::macros::fit_size(&mut converted.mac)); - // The shortcut, unless another macro took it while recording. - if let Some(k) = &start.shortcut - && crate::macros::parse_shortcut(k).is_some_and(|c| crate::macros::shortcut_conflict(&wb, &name, c).is_none()) - { - converted.mac.shortcut = Some(k.clone()); + // The shortcut, unless another macro took it while recording; then it is left off and the + // result says so. + let mut shortcut_dropped = None; + if let Some(k) = &start.shortcut { + if crate::macros::parse_shortcut(k).is_some_and(|c| crate::macros::shortcut_conflict(&wb, &name, c).is_none()) { + converted.mac.shortcut = Some(k.clone()); + } else { + shortcut_dropped = Some(k.clone()); + } } let steps = converted.mac.steps.len(); let inputs: Vec = converted.mac.inputs.iter().map(|i| i.name.clone()).collect(); @@ -360,7 +364,9 @@ fn save_recording(s: &mut Session, p: &Json, rec: &crate::record::Recorder, star let key = (doc, crate::macros::fingerprint(m)); s.macro_approvals.insert(key); } - Ok( - json!({"recording": false, "saved": name, "steps": steps, "dropped": rec.dropped.saturating_add(cut), "skipped": converted.skipped, "inputs": inputs}), - ) + let mut out = json!({"recording": false, "saved": name, "steps": steps, "dropped": rec.dropped.saturating_add(cut), "skipped": converted.skipped, "inputs": inputs}); + if let Some(k) = shortcut_dropped { + out["shortcutDropped"] = json!(k); + } + Ok(out) } diff --git a/crates/engine/src/tests/macros.rs b/crates/engine/src/tests/macros.rs index 868356c41..19a23a1ac 100644 --- a/crates/engine/src/tests/macros.rs +++ b/crates/engine/src/tests/macros.rs @@ -2413,3 +2413,22 @@ fn shortcuts_parse_to_one_letter_and_avoid_reserved_and_taken_ones() { assert_eq!(shortcut_conflict(&wb, "a", 'M'), None); assert_eq!(shortcut_conflict(&wb, "B", 'Q'), None); } + +#[test] +fn a_shortcut_taken_while_recording_is_reported_when_it_is_left_off() { + let mut s = s(); + s.execute("macro.record", json!({"name": "Tidy", "shortcut": "m"})).unwrap(); + set(&mut s, "A1", "1"); + // Another macro takes Ctrl+Shift+M before recording stops. + s.execute("macro.save", json!({"name": "Other", "shortcut": "Ctrl+Shift+M", "steps": []})).unwrap(); + let r = s.execute("macro.record", json!({"stop": true})).unwrap(); + assert_eq!(r["saved"], "Tidy"); + assert_eq!(r["shortcutDropped"], "Ctrl+Shift+M", "{r}"); + assert!(s.execute("macro.get", json!({"name": "Tidy"})).unwrap()["shortcut"].is_null()); + // A free one is kept and not reported. + s.execute("macro.record", json!({"name": "Keep", "shortcut": "q"})).unwrap(); + set(&mut s, "A2", "2"); + let r = s.execute("macro.record", json!({"stop": true})).unwrap(); + assert!(r.get("shortcutDropped").is_none(), "{r}"); + assert_eq!(s.execute("macro.get", json!({"name": "Keep"})).unwrap()["shortcut"], "Ctrl+Shift+Q"); +} diff --git a/crates/ui-egui/src/macro_ui.rs b/crates/ui-egui/src/macro_ui.rs index dcf691f65..d80e4c800 100644 --- a/crates/ui-egui/src/macro_ui.rs +++ b/crates/ui-egui/src/macro_ui.rs @@ -89,7 +89,11 @@ pub fn stop_recording(app: &mut SheetApp) { Some(name) => { let steps = v["steps"].as_u64().unwrap_or(0); app.macros.note_recent(name); - app.toast = Some((format!("Saved macro “{name}” ({steps} step{})", if steps == 1 { "" } else { "s" }), crate::now_ms())); + let mut text = format!("Saved macro “{name}” ({steps} step{})", if steps == 1 { "" } else { "s" }); + if let Some(k) = v["shortcutDropped"].as_str() { + text.push_str(&format!(" without {k}: another macro took it while recording")); + } + app.toast = Some((text, crate::now_ms())); } None => app.toast = Some(("Recording stopped".into(), crate::now_ms())), }, diff --git a/docs/macros.md b/docs/macros.md index 4f2051a0b..5018975d6 100644 --- a/docs/macros.md +++ b/docs/macros.md @@ -198,19 +198,19 @@ on its own, never as a step of a script or macro, and is never recorded. references) and, from the Record Macro dialog, `name`, `shortcut` and `description`: the macro's name, its Ctrl+Shift shortcut letter (`"Ctrl+Shift+K"` or `"K"`) and its description. They are checked at once, so a name that is taken (a kept one included), too long or holds line breaks, or a -letter that GridCraft or another macro uses, is refused before anything is recorded. Without a -name the macro is called `Macro1`, `Macro2` and so on. A `name` or `description` given when -stopping wins over the one given when starting; a name that another macro has by then gets a -suffix, such as `Tidy (2)`, so stopping never replaces a macro, and a shortcut another macro took -meanwhile is left off, which the result says in `shortcutDropped`. `cancel: true` ends a recording -without saving it, the way out of one whose macro can't be saved. A parameter the call would ignore is refused: `relative` or `shortcut` when -stopping, `stop: false` while recording, anything beside `cancel: true`, and one of the wrong kind. -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 recorded. The person -recorded the macro, so in a workbook opened from a file it runs without the approval a macro that -came with the file needs, until it changes. +letter that GridCraft or another macro uses, is refused before anything is recorded. Without a name +the macro is called `Macro1`, `Macro2` and so on. A `name` or `description` given when stopping wins +over the one given when starting; a name that another macro has by then gets a suffix, such as `Tidy +(2)`, so stopping never replaces a macro, and a shortcut another macro took meanwhile is left off, +which the result says in `shortcutDropped`. `cancel: true` ends a recording without saving it, the +way out of one whose macro can't be saved. A parameter the call would ignore is refused: `relative` +or `shortcut` when stopping, `stop: false` while recording, anything beside `cancel: true`, and one +of the wrong kind. 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 recorded. The person recorded +the macro, so in a workbook opened from a file it runs without the approval a macro that came with +the file needs, until it changes. A recording keeps up to 100,000 commands and 64 MB of them as JSON, and a macro holds up to 10,000 steps and 4,194,304 characters of JSON. The steps past any of these limits, and steps