diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 856f1777e..b2d4af09e 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -622,8 +622,12 @@ impl Session { // finds no recorder. A menu-style call that only opened a dialog (`file.open` with no // file, `home.formatCells` with no style) is not recorded either: the dialog's own // call is, so playing the recording back runs the command once, with its parameters. + // A call that opened a dialog and changed the workbook too (a script with an edit before + // a menu-style step) is recorded: the edit happened, and the dialog's confirming call + // is a step of its own. let opened_dialog = self.ui_requests.get(requests_before..).is_some_and(|new| new.iter().any(|q| matches!(q, UiRequest::Dialog(..)))); - if let Some((doc, sheet, sheet_name, selection)) = place.filter(|_| !opened_dialog && !record::NOT_RECORDED.contains(&id)) + let changed = before.as_ref().is_some_and(|(uid, wb, _)| self.docs.iter().any(|d| d.uid == *uid && !Arc::ptr_eq(&d.wb, wb))); + if let Some((doc, sheet, sheet_name, selection)) = place.filter(|_| (!opened_dialog || changed) && !record::NOT_RECORDED.contains(&id)) && let Some(rec) = self.recorder.as_mut() { rec.push(record::RecordedStep { doc, sheet, sheet_name, selection, id: id.to_string(), params: cmd::journal_params(¶ms) }); diff --git a/crates/engine/src/tests/seams.rs b/crates/engine/src/tests/seams.rs index 45d57e6f6..e93116120 100644 --- a/crates/engine/src/tests/seams.rs +++ b/crates/engine/src/tests/seams.rs @@ -846,6 +846,26 @@ fn stop_action_runs_only_on_its_own_and_is_not_recorded() { assert_eq!(s.stop_recording().unwrap().dropped, 0); } +#[test] +fn a_run_that_edits_and_opens_a_dialog_is_recorded() { + // A script with an edit before a menu-style step: the edit happened, so the run is a step of + // the recording (the person's confirming call is another). Only a call that opened a dialog + // and changed nothing is left out. + let mut s = s(); + let edit_then_dialog = json!([{"command": "cell.set", "params": {"cell": "B1", "input": "1"}}, {"command": "home.formatCells", "params": {}}]); + let dialog_only = json!([{"command": "home.formatCells", "params": {}}]); + s.execute("automate.newScript", json!({"name": "Edit", "commands": edit_then_dialog})).unwrap(); + s.execute("automate.newScript", json!({"name": "Ask", "commands": dialog_only})).unwrap(); + s.start_recording(); + s.execute("automate.runScript", json!({"name": "Ask"})).unwrap(); + assert!(s.take_ui_requests().iter().any(|q| matches!(q, crate::UiRequest::Dialog(..)))); + assert!(recorded_ids(&s).is_empty(), "{:?}", recorded_ids(&s)); + s.execute("automate.runScript", json!({"name": "Edit"})).unwrap(); + assert!(s.take_ui_requests().iter().any(|q| matches!(q, crate::UiRequest::Dialog(..)))); + assert_eq!(recorded_ids(&s), ["automate.runScript"], "the edit happened, so the run is a step"); + assert_eq!(s.stop_recording().unwrap().dropped, 0); +} + #[test] fn the_recorder_keeps_top_level_calls_with_where_they_ran() { let mut s = s(); diff --git a/docs/architecture.md b/docs/architecture.md index 60e095611..34c167c39 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -72,7 +72,8 @@ Actions made of many commands (macros, a debugger that runs one step per call) u - **Recording buffer** (`record.rs`). `Session::start_recording` collects each successful top-level command with the document, sheet and selection it ran in, without secret parameters. Commands run by other commands are not recorded, nor is a menu-style call that only opened a - dialog (the dialog's confirming call is the step), and the buffer is independent of the journal + dialog (the dialog's confirming call is the step; a run that changed the workbook before a step + of it opened a dialog is recorded), and the buffer is independent of the journal (Automate › Record Actions still records through the journal). - **Document trust.** `DocState::trust` is `Local` for workbooks made in the session and `FromFile` for workbooks opened from a file. `Trust` has no default: `DocState::new` takes it,