diff --git a/crates/engine/src/cmd/macros.rs b/crates/engine/src/cmd/macros.rs index 71dfbef67..de1edd367 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?, 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?, 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}", has_doc, record ), @@ -211,13 +211,13 @@ fn record(s: &mut Session, p: &Json) -> Result { if bool_param(p, "stop").unwrap_or(false) || s.is_recording() { return stop_recording(s, p); } - start_recording(s, bool_param(p, "relative").unwrap_or(true)) + start_recording(s, p, bool_param(p, "relative").unwrap_or(true)) } -/// Refuses parameters `macro.record` would ignore (`null` counts as not given): `name` and -/// `description` when starting (they name the macro when it stops), `relative` when stopping (it -/// is chosen when recording starts), `stop: false` while recording (the call stops it), anything -/// beside `cancel: true`, and parameters of the wrong kind. +/// Refuses parameters `macro.record` would ignore (`null` counts as not given): `relative` and +/// `shortcut` when stopping (they are chosen when recording starts), `stop: false` while +/// recording (the call stops it), anything beside `cancel: true`, and parameters of the wrong +/// kind. `name` and `description` count when starting and when stopping (stopping wins). fn check_record_params(recording: bool, p: &Json) -> Result<()> { let given = |k: &str| p.get(k).filter(|v| !v.is_null()); for k in ["stop", "cancel", "relative"] { @@ -225,13 +225,13 @@ fn check_record_params(recording: bool, p: &Json) -> Result<()> { return Err(bad("macro.record", format!("`{k}` must be true or false"))); } } - for k in ["name", "description"] { + for k in ["name", "description", "shortcut"] { if given(k).is_some_and(|v| !v.is_string()) { return Err(bad("macro.record", format!("`{k}` must be text"))); } } if given("cancel").and_then(Json::as_bool) == Some(true) { - if let Some(k) = ["stop", "relative", "name", "description"].into_iter().find(|k| given(k).is_some()) { + if let Some(k) = ["stop", "relative", "name", "description", "shortcut"].into_iter().find(|k| given(k).is_some()) { return Err(bad("macro.record", format!("`cancel: true` ends the recording without saving it, so `{k}` would be ignored"))); } return Ok(()); @@ -243,22 +243,50 @@ fn check_record_params(recording: bool, p: &Json) -> Result<()> { "a recording is in progress, and this call stops it: pass `stop: true`, or `cancel: true` to end it unsaved", )); } - if recording || stop == Some(true) { - if given("relative").is_some() { - return Err(bad("macro.record", "`relative` is chosen when recording starts")); - } - } else if let Some(k) = ["name", "description"].into_iter().find(|k| given(k).is_some()) { - return Err(bad("macro.record", format!("`{k}` names the macro when recording stops, not when it starts"))); + if (recording || stop == Some(true)) + && let Some(k) = ["relative", "shortcut"].into_iter().find(|k| given(k).is_some()) + { + return Err(bad("macro.record", format!("`{k}` is chosen when recording starts"))); } Ok(()) } -fn start_recording(s: &mut Session, relative: bool) -> Result { - let start = RecordingStart::new(s.doc()?, relative); +/// Starts recording. `name`, `shortcut` and `description` (from the Record Macro dialog) are +/// checked now, so a bad name or a taken shortcut is refused before anything is recorded. +fn start_recording(s: &mut Session, p: &Json, relative: bool) -> Result { + let d = s.doc()?; + let name = match str_param(p, "name").map(str::trim).filter(|n| !n.is_empty()) { + Some(n) => Some(crate::macros::check_new_name(&d.wb, n).map_err(|e| bad("macro.record", e))?), + None => None, + }; + let shortcut = match str_param(p, "shortcut").map(str::trim).filter(|k| !k.is_empty()) { + Some(k) => { + let letter = crate::macros::parse_shortcut(k) + .ok_or_else(|| bad("macro.record", "`shortcut` must be one letter from A to Z (used as Ctrl+Shift+letter)"))?; + if let Some(why) = crate::macros::shortcut_conflict(&d.wb, name.as_deref().unwrap_or(""), letter) { + return Err(bad("macro.record", why)); + } + Some(crate::macros::shortcut_text(letter)) + } + None => None, + }; + let description = match str_param(p, "description") { + Some(t) if t.chars().count() > gridcraft_model::MAX_MACRO_TEXT => { + return Err(bad("macro.record", format!("`description` can be at most {} characters long", gridcraft_model::MAX_MACRO_TEXT))); + } + Some(t) => Some(t.to_string()), + None => None, + }; + let mut start = RecordingStart::new(d, relative); + (start.name, start.shortcut, start.description) = (name.clone(), shortcut, description); let input = start.selection.current().a1(); s.start_recording(); s.macro_recording = Some(start); - Ok(if relative { json!({"recording": true, "relative": true, "input": input}) } else { json!({"recording": true, "relative": false}) }) + let mut out = if relative { json!({"recording": true, "relative": true, "input": input}) } else { json!({"recording": true, "relative": false}) }; + if let Some(n) = name { + out["name"] = json!(n); + } + Ok(out) } /// Stops recording and saves what was recorded as a macro, in the workbook where recording @@ -295,8 +323,9 @@ fn save_recording(s: &mut Session, p: &Json, rec: &crate::record::Recorder, star // input from the workbook as it is now. let start = start.unwrap_or_else(|| RecordingStart::new(d, true)); let (doc, wb) = (d.uid, 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()) { + // A name given when stopping wins over the one given when starting; either one, if another + // macro has it, gets a suffix, so the save never replaces a macro. + let name = match str_param(p, "name").map(str::trim).filter(|n| !n.is_empty()).or(start.name.as_deref()) { Some(n) => crate::macros::unique_name(&wb, n), None => crate::macros::next_name(&wb), }; @@ -305,10 +334,16 @@ fn save_recording(s: &mut Session, p: &Json, rec: &crate::record::Recorder, star // out and counted with the ones the recorder dropped. let mut 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") { + if let Some(desc) = str_param(p, "description").or(start.description.as_deref()) { 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()); + } let steps = converted.mac.steps.len(); let inputs: Vec = converted.mac.inputs.iter().map(|i| i.name.clone()).collect(); // Saved into that workbook even when another one is active now. diff --git a/crates/engine/src/macros/mod.rs b/crates/engine/src/macros/mod.rs index 75d96f0df..f500b9815 100644 --- a/crates/engine/src/macros/mod.rs +++ b/crates/engine/src/macros/mod.rs @@ -420,6 +420,67 @@ impl Taken { } } +/// Ctrl+Shift (Cmd+Shift on a Mac) letters that GridCraft already uses, on any platform, which a +/// macro cannot take. +pub const RESERVED_SHORTCUTS: &[(char, &str)] = &[ + ('F', "Format Cells (Font) or Filter"), + ('G', "Workbook Statistics"), + ('H', "Replace"), + ('J', "Ungroup"), + ('K', "Group"), + ('L', "Filter"), + ('R', "Sort"), + ('S', "Save As"), + ('T', "AutoSum"), + ('Z', "Redo"), +]; + +/// The letter of a macro shortcut: `"Ctrl+Shift+M"`, `"⌘⇧M"` and `"m"` all give `'M'`. `None` +/// when it is not one letter from A to Z. +pub fn parse_shortcut(s: &str) -> Option { + let last = s.trim().rsplit(['+', '⇧', '⌘']).next()?.trim(); + let mut chars = last.chars(); + match (chars.next(), chars.next()) { + (Some(c), None) if c.is_ascii_alphabetic() => Some(c.to_ascii_uppercase()), + _ => None, + } +} + +/// How a shortcut is stored and shown: `Ctrl+Shift+M`. +pub fn shortcut_text(letter: char) -> String { + format!("Ctrl+Shift+{}", letter.to_ascii_uppercase()) +} + +/// Why macro `name` cannot use Ctrl+Shift+`letter`: GridCraft uses it, or another macro has it. +pub fn shortcut_conflict(wb: &Workbook, name: &str, letter: char) -> Option { + let letter = letter.to_ascii_uppercase(); + if let Some((_, what)) = RESERVED_SHORTCUTS.iter().find(|(c, _)| *c == letter) { + return Some(format!("{} is {what}. Choose another letter.", shortcut_text(letter))); + } + wb.macros + .iter() + .find(|m| !m.name.eq_ignore_ascii_case(name.trim()) && m.shortcut.as_deref().and_then(parse_shortcut) == Some(letter)) + .map(|m| format!("{} already runs macro “{}”. Choose another letter.", shortcut_text(letter), m.name)) +} + +/// `name` trimmed, if it can name a new macro in `wb`; otherwise why not: it is empty, too long, +/// holds line breaks or another macro has it. +pub fn check_new_name(wb: &Workbook, name: &str) -> Result { + let name = name.trim(); + let probe = Macro { name: name.to_string(), ..Macro::default() }; + probe.check_limits()?; + if name.chars().any(char::is_control) { + return Err("A macro name cannot hold line breaks or other control characters.".into()); + } + if find(wb, name).is_some() { + return Err(format!("A macro called “{name}” already exists. Choose another name.")); + } + if let Some(kept) = kept_name(wb, name) { + return Err(format!("A macro called “{kept}” is kept from a newer version of GridCraft. Choose another name.")); + } + Ok(name.to_string()) +} + /// The first free `Macro1`, `Macro2`… name, also free among the kept macros ([`kept_name`]). pub fn next_name(wb: &Workbook) -> String { (1..).map(|n| format!("Macro{n}")).find(|n| find(wb, n).is_none() && kept_name(wb, n).is_none()).unwrap_or_else(|| "Macro".into()) diff --git a/crates/engine/src/macros/recorder.rs b/crates/engine/src/macros/recorder.rs index 53b8edc30..98249cb84 100644 --- a/crates/engine/src/macros/recorder.rs +++ b/crates/engine/src/macros/recorder.rs @@ -32,7 +32,8 @@ use crate::record::RecordedStep; const NEAR_ROWS: u32 = 50; const NEAR_COLS: u32 = 26; -/// Where recording started, kept beside the buffer. +/// Where recording started, kept beside the buffer, with the name, shortcut and description the +/// macro is to be saved with (from the Record Macro dialog), applied when recording stops. #[derive(Clone, Debug)] pub struct RecordingStart { pub doc: u64, @@ -44,6 +45,10 @@ pub struct RecordingStart { /// started: the recorded steps may change its values (text made into numbers), and the /// macro is for data like the data it was recorded on. pub input: Option, + /// The name, shortcut and description given when recording started (Record Macro). + pub name: Option, + pub shortcut: Option, + pub description: Option, } impl RecordingStart { @@ -57,6 +62,9 @@ impl RecordingStart { selection: d.selection.clone(), relative, input: relative.then(|| infer_input(&d.wb, sheet, d.selection.current())), + name: None, + shortcut: None, + description: None, } } } diff --git a/crates/engine/src/tests/macros.rs b/crates/engine/src/tests/macros.rs index 459faa406..868356c41 100644 --- a/crates/engine/src/tests/macros.rs +++ b/crates/engine/src/tests/macros.rs @@ -2077,6 +2077,9 @@ fn kept_macros_count_toward_the_limit_and_keep_their_names() { let err = s.execute(cmd, p).unwrap_err().to_string(); assert!(err.contains("newer version"), "{cmd}: {err}"); } + // Record Macro refuses a kept macro's name when recording starts. + let err = s.execute("macro.record", json!({"name": "future"})).unwrap_err().to_string(); + assert!(err.contains("newer version") && !s.is_recording(), "{err}"); // A recording takes the next free name, around the kept one, and fills the last place. s.execute("macro.record", json!({"relative": false})).unwrap(); set(&mut s, "A1", "typed"); @@ -2318,12 +2321,13 @@ fn a_recording_past_the_macro_size_is_cut_and_counted() { #[test] fn record_refuses_parameters_it_would_ignore() { let mut s = s(); - for p in [json!({"name": "Early"}), json!({"description": "d"}), json!({"relative": "no"}), json!({"stop": 1})] { + // `name`, `description` and `shortcut` count when starting (Record Macro); wrong kinds don't. + for p in [json!({"name": 5}), json!({"shortcut": true}), json!({"relative": "no"}), json!({"stop": 1})] { assert!(s.execute("macro.record", p.clone()).is_err(), "{p}"); assert!(!s.is_recording(), "{p}"); } s.execute("macro.record", json!({"relative": false, "name": null})).unwrap(); - for p in [json!({"relative": true}), json!({"stop": false}), json!({"cancel": true, "name": "x"}), json!({"name": 3})] { + for p in [json!({"relative": true}), json!({"shortcut": "K"}), json!({"stop": false}), json!({"cancel": true, "name": "x"}), json!({"name": 3})] { assert!(s.execute("macro.record", p.clone()).is_err(), "{p}"); assert!(s.is_recording(), "{p}"); } @@ -2354,3 +2358,58 @@ fn undo_while_recording_keeps_the_undone_step_as_excel_does() { s.execute("macro.run", json!({"name": "Rec", "onConflict": "overwrite"})).unwrap(); assert_eq!(v(&s, "A1").display(), "typed"); } + +#[test] +fn record_checks_a_name_shortcut_and_description_when_it_starts() { + let mut s = s(); + s.execute("macro.save", json!({"name": "Taken", "shortcut": "Ctrl+Shift+P", "steps": []})).unwrap(); + for (p, needle) in [ + (json!({"name": "taken"}), "already exists"), + (json!({"name": "a\nb"}), "line breaks"), + (json!({"name": "x".repeat(300)}), "at most"), + (json!({"name": "New", "shortcut": "p"}), "already runs macro"), + (json!({"name": "New", "shortcut": "Ctrl+Shift+Z"}), "Redo"), + (json!({"name": "New", "shortcut": "F5"}), "one letter"), + (json!({"name": "New", "description": "d".repeat(100_000)}), "at most"), + ] { + let e = s.execute("macro.record", p.clone()).unwrap_err().to_string(); + assert!(e.contains(needle), "{p}: {e}"); + assert!(!s.is_recording(), "{p}"); + } + let r = s.execute("macro.record", json!({"name": " Tidy ", "shortcut": "j", "description": "Tidies"})); + // J is Ungroup on a Mac, so it is reserved; M is free. + assert!(r.is_err_and(|e| e.to_string().contains("Ungroup"))); + let r = s.execute("macro.record", json!({"name": " Tidy ", "shortcut": "m", "description": "Tidies"})).unwrap(); + assert_eq!(r["name"], json!("Tidy")); + set(&mut s, "A1", "1"); + let r = s.execute("macro.record", json!({"stop": true})).unwrap(); + assert_eq!(r["saved"], json!("Tidy")); + let m = s.execute("macro.get", json!({"name": "Tidy"})).unwrap(); + assert_eq!((m["shortcut"].clone(), m["description"].clone()), (json!("Ctrl+Shift+M"), json!("Tidies"))); + // Empty params record under the next MacroN, as before, and never open a dialog. + s.execute("macro.record", json!({})).unwrap(); + set(&mut s, "A2", "2"); + let r = s.execute("macro.record", json!({})).unwrap(); + assert!(r["saved"].as_str().is_some_and(|n| n.starts_with("Macro")), "{r}"); + assert!(s.take_ui_requests().is_empty(), "no dialog"); +} + +#[test] +fn shortcuts_parse_to_one_letter_and_avoid_reserved_and_taken_ones() { + use crate::macros::{parse_shortcut, shortcut_conflict, shortcut_text}; + assert_eq!(parse_shortcut("Ctrl+Shift+M"), Some('M')); + assert_eq!(parse_shortcut("⌘⇧k"), Some('K')); + assert_eq!(parse_shortcut(" q "), Some('Q')); + for bad in ["Ctrl+Shift+F5", "Ctrl+Shift+", "", "Ctrl+Shift+é"] { + assert_eq!(parse_shortcut(bad), None, "{bad}"); + } + assert_eq!(shortcut_text('m'), "Ctrl+Shift+M"); + let mut wb = gridcraft_model::Workbook::default(); + wb.macros.push(gridcraft_model::Macro { name: "A".into(), shortcut: Some("Ctrl+Shift+M".into()), ..Default::default() }); + wb.macros.push(gridcraft_model::Macro { name: "B".into(), ..Default::default() }); + assert!(shortcut_conflict(&wb, "B", 's').is_some_and(|m| m.contains("Save As"))); + assert!(shortcut_conflict(&wb, "B", 'm').is_some_and(|m| m.contains("“A”"))); + // A macro keeps its own letter. + assert_eq!(shortcut_conflict(&wb, "a", 'M'), None); + assert_eq!(shortcut_conflict(&wb, "B", 'Q'), None); +} diff --git a/docs/macros.md b/docs/macros.md index 6ab45053e..f23358511 100644 --- a/docs/macros.md +++ b/docs/macros.md @@ -1,6 +1,6 @@ # Macros -> **Last updated:** 2026-10-25 · **Change:** minor (recording: kept steps, the input as it was, text that stays text, size limit, parameters refused; saving needs `replace: true` to replace; kept macros count and keep their names; earlier: recording macros and the management commands) +> **Last updated:** 2026-10-25 · **Change:** minor (Record Macro takes a name, shortcut and description; recording: kept steps, the input as it was, text that stays text, size limit, parameters refused; saving needs `replace: true` to replace; kept macros count and keep their names; earlier: recording macros and the management commands) GridCraft macros are typed, declarative lists of commands, not VBA. Every step runs a GridCraft command (the same ids the ribbon, CLI and MCP use) or one of three bounded control steps. A macro @@ -195,12 +195,17 @@ on its own, never as a step of a script or macro, and is never recorded. ## Recording `macro.record` toggles recording. Starting takes `relative` (default true; `false` for absolute -references); stopping takes `name` and `description`. A name that another macro has, a kept one -included, gets a suffix, such as `Tidy (2)`, so stopping never replaces a macro. `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: `name` or `description` when starting, `relative` when -stopping, `stop: false` while recording, `stop`, `relative`, `name` or `description` 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; +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. `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 @@ -231,6 +236,10 @@ recording's 64 MB is left out and listed. Undo while recording works as in Excel: `edit.undo` is left out (a macro never runs it), and the step it undid stays in the recording, so playing the recording back makes that edit again. +GridCraft keeps the Ctrl+Shift letters it uses itself on some platform: F, G, H, J, K, L, R, S, T +and Z (`macros::RESERVED_SHORTCUTS`). A shortcut a file brings with it on one of these letters is +kept but does not run the macro. + ## Other commands and storage `macro.list`, `macro.get`, `macro.save {macro, replace?}`, `macro.delete` and `macro.rename