Merge the footprint pull request (24f0c3df, with #2144 and #2151) into #2145

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.

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
+    /// Takes back the steps recorded in the document with uid `doc` since `mark` (an undo group
-    /// back, so playing them back would make edits the workbook no longer has).
+    /// on it was rolled back, so playing them back would make edits it no longer has). Steps in
-    pub(crate) fn cut_back(&mut self, (len, dropped): (usize, usize)) {
+    /// other documents stay: their edits do too. Steps the buffer dropped stay counted.
-        if self.steps.len() > len {
+    pub(crate) fn cut_back(&mut self, (len, _): (usize, usize), doc: u64) {
-            self.steps.truncate(len);
+        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);
     }
 }

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

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);
 }