diff --git a/crates/engine/src/cmd/macros.rs b/crates/engine/src/cmd/macros.rs index 426d06492..6cfd069f7 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; 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 { } fn record(s: &mut Session, p: &Json) -> Result { + // `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(); diff --git a/crates/engine/src/macros/recorder.rs b/crates/engine/src/macros/recorder.rs index 210872475..257278b02 100644 --- a/crates/engine/src/macros/recorder.rs +++ b/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, + /// 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 = 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(¶ms, 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> = 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 { /// 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 { + 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 { diff --git a/crates/engine/src/tests/macros.rs b/crates/engine/src/tests/macros.rs index df6e59606..3e246ee71 100644 --- a/crates/engine/src/tests/macros.rs +++ b/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 = (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(); diff --git a/docs/macros.md b/docs/macros.md index 503068707..22742ac85 100644 --- a/docs/macros.md +++ b/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