diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 041e5d65c..43f396c41 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -620,6 +620,9 @@ impl Session { return Err(EngineError::Disabled(id.to_string(), macros::debug::BUSY.into())); } let before = self.active().map(|d| (d.uid, d.wb.clone(), d.selection.clone())); + // A dry run inside this command that a panic cuts short leaves journaling paused and its + // touch log open; the panic branch puts both back. + let paused_before = self.journal_paused; let params = if params.is_null() { Json::Object(Default::default()) } else { params }; let (first_group, group_epoch) = (self.next_group, self.group_epoch); // Whether a group was open on the document before the command, and how long its history was. @@ -686,14 +689,18 @@ impl Session { } d.calc.rebuild(&d.wb); } + if self.journal_paused != paused_before { + self.journal_paused = paused_before; + for d in self.docs.iter_mut() { + d.touched = None; + } + } if self.depth == 0 { // Only the `macro.debug` call itself can lose the run's place: a command nested // in a step that panics is an error of that step, which the run handles. macros::debug::recover(self); self.trim_history(); self.group_step = None; - // A dry run the panic cut short never put journaling back on. - self.journal_paused = 0; } return Err(EngineError::Internal(id.to_string(), msg)); } @@ -936,7 +943,7 @@ impl Session { // The steps recorded while the group was open go with their edits (all of them when the // recording started inside the group). if let Some(rec) = self.recorder.as_mut() { - rec.cut_back(g.recorded.unwrap_or((0, 0))); + rec.cut_back(g.recorded.unwrap_or((0, 0)), g.doc); } // The record of the last group step stays when the document is back exactly where that // step left it (this group began there); otherwise the rollback is where the group ended. diff --git a/crates/engine/src/record.rs b/crates/engine/src/record.rs index 913e4f86f..4fea4620e 100644 --- a/crates/engine/src/record.rs +++ b/crates/engine/src/record.rs @@ -74,13 +74,18 @@ impl Recorder { (self.steps.len(), self.dropped) } - /// Takes back the steps recorded since `mark` (an undo group that recorded them was rolled - /// back, so playing them back would make edits the workbook no longer has). - pub(crate) fn cut_back(&mut self, (len, dropped): (usize, usize)) { - if self.steps.len() > len { - self.steps.truncate(len); + /// Takes back the steps recorded in the document with uid `doc` since `mark` (an undo group + /// on it was rolled back, so playing them back would make edits it no longer has). Steps in + /// other documents stay: their edits do too. Steps the buffer dropped stay counted. + pub(crate) fn cut_back(&mut self, (len, _): (usize, usize), doc: u64) { + let before = self.steps.len(); + let mut i = 0; + self.steps.retain(|s| { + i += 1; + i <= len || s.doc != doc + }); + if self.steps.len() != before { self.bytes = self.steps.iter().map(|s| serde_json::to_vec(s).map_or(0, |v| v.len())).sum(); } - self.dropped = self.dropped.min(dropped); } } diff --git a/crates/engine/src/tests/macros.rs b/crates/engine/src/tests/macros.rs index 941a4a29f..9aef09366 100644 --- a/crates/engine/src/tests/macros.rs +++ b/crates/engine/src/tests/macros.rs @@ -717,6 +717,27 @@ fn a_dry_run_leaves_a_clean_workbook_clean() { assert!(!s.doc().unwrap().is_dirty()); } +#[test] +fn a_dry_run_cut_short_by_a_panic_leaves_journaling_on() { + // A panic inside a command that started a dry run, nested in another command: the panic + // guard of the nested call puts the pause and the touch log back, so the session journals + // the person's commands again. + crate::cmd::register_undoable_test_command("test.dryRunThenPanic", |s, _| { + s.journal_paused = s.journal_paused.saturating_add(1); + if let Ok(d) = s.doc_mut() { + d.touched = Some(Vec::new()); + } + panic!("in a dry run"); + }); + let mut s = s(); + let err = s.execute_nested("test.dryRunThenPanic", json!({})).unwrap_err().to_string(); + assert!(err.contains("in a dry run"), "{err}"); + assert_eq!((s.journal_paused, s.doc().unwrap().touched.is_none()), (0, true)); + let n = s.journal.len(); + s.execute("selection.set", json!({"range": "B2"})).unwrap(); + assert_eq!(s.journal.len(), n + 1, "journaled again"); +} + #[test] fn a_dry_run_leaves_the_journal_and_the_recording_start_as_they_were() { // A dry run journals nothing: journaling its nested commands would push a full journal past diff --git a/crates/engine/src/tests/seams.rs b/crates/engine/src/tests/seams.rs index 48d5abf4c..3fa37b100 100644 --- a/crates/engine/src/tests/seams.rs +++ b/crates/engine/src/tests/seams.rs @@ -844,6 +844,16 @@ fn stop_action_runs_only_on_its_own_and_is_not_recorded() { assert_eq!(s.execute("automate.stopAction", json!({})).unwrap(), json!({"stopped": true})); assert!(v(&s, "A3").is_empty()); assert_eq!(recorded_ids(&s), ["cell.set", "cell.set"]); + // An edit recorded in another workbook while the group was open stays, as its edit does. + let first = s.active_index(); + let _left = s.group_begin("Left open").unwrap(); + set(&mut s, "A4", "four"); + s.new_workbook(); + set(&mut s, "B1", "other"); + s.set_active(first); + assert_eq!(s.execute("automate.stopAction", json!({})).unwrap(), json!({"stopped": true})); + assert_eq!(recorded_ids(&s), ["cell.set", "cell.set", "cell.set"], "the other workbook's edit stays"); + assert_eq!(s.recorder.as_ref().unwrap().steps.last().map(|st| st.params["cell"].clone()), Some(json!("B1"))); assert_eq!(s.stop_recording().unwrap().dropped, 0); }