diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index daea7f7e8..e492c4dd6 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -599,6 +599,9 @@ impl Session { return Err(EngineError::Disabled(id.to_string(), why)); } 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. @@ -666,11 +669,15 @@ 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 { 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)); } diff --git a/crates/engine/src/tests/macros.rs b/crates/engine/src/tests/macros.rs index 5df8faa11..153c832ca 100644 --- a/crates/engine/src/tests/macros.rs +++ b/crates/engine/src/tests/macros.rs @@ -417,6 +417,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