From 2e8284c191aa575f29d0da4a734ad580d82e0baa Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Sun, 25 Oct 2026 06:26:51 -0400 Subject: [PATCH] Engine: a panic during a dry run puts the journal pause and the touch log back at any depth The pause of a dry run was cleared only by a panic at the top level. A command that started a dry run, nested in another, and panicked left journaling paused for the rest of the session (and the touch log open) once the outer command finished. Each command now keeps the pause count it started with, and its panic guard puts it back, closing the touch logs, whatever its depth. - Test: `a_dry_run_cut_short_by_a_panic_leaves_journaling_on`. --- crates/engine/src/lib.rs | 11 +++++++++-- crates/engine/src/tests/macros.rs | 21 +++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) 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