diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index bf1100082..aaa8db4f6 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -533,8 +533,9 @@ impl Session { // Whether a group was open on the document before the command, and how long its history was. let start = before.as_ref().and_then(|(uid, ..)| self.docs.iter().find(|d| d.uid == *uid).map(|d| (self.in_group(*uid), d.undo.len()))); // A command that panics leaves the history as it found it: the steps commands nested in it - // made go with the workbook, and the redo history they cleared comes back. - let redo_before = before.as_ref().and_then(|(uid, ..)| self.docs.iter().find(|d| d.uid == *uid).map(|d| d.redo.clone())); + // made or undid, and the redo history they cleared, come back. A length is not enough, + // because a nested `edit.undo` removes entries. + let history_before = before.as_ref().and_then(|(uid, ..)| self.docs.iter().find(|d| d.uid == *uid).map(|d| (d.undo.clone(), d.redo.clone()))); // The recorder keeps top-level calls with the place they ran in. let place = (self.depth == 0 && self.recorder.is_some()).then(|| match self.active() { Some(d) => (d.uid, d.wb.active_sheet, d.wb.active().map(|sh| sh.name.clone()).unwrap_or_default(), d.selection.clone()), @@ -562,10 +563,8 @@ impl Session { { d.wb = wb.clone(); d.selection = sel.clone(); - if let Some((_, len)) = start { - d.undo.truncate(len); - } - if let Some(redo) = redo_before { + if let Some((undo, redo)) = history_before { + d.undo = undo; d.redo = redo; } d.calc.rebuild(&d.wb); diff --git a/crates/engine/src/tests/seams.rs b/crates/engine/src/tests/seams.rs index cb0902cdf..9c9aeb6ad 100644 --- a/crates/engine/src/tests/seams.rs +++ b/crates/engine/src/tests/seams.rs @@ -563,6 +563,20 @@ fn a_command_that_panics_leaves_the_history_as_it_found_it() { assert_eq!(s.doc().unwrap().redo.len(), 1, "the redo history is back"); s.execute("edit.redo", json!({})).unwrap(); assert_eq!(v(&s, "A1").display(), "2"); + + // A command that undoes through a nested command and then panics: the entry Undo removed + // comes back, and the redo entry it made goes. + crate::cmd::register_undoable_test_command("test.undoThenPanic", |s, _| { + s.execute("edit.undo", json!({}))?; + panic!("after an undo"); + }); + assert_eq!((undo_labels(&s).len(), s.doc().unwrap().redo.len()), (2, 0)); + assert!(s.execute("test.undoThenPanic", json!({})).is_err()); + assert_eq!(v(&s, "A1").display(), "2", "the workbook is restored"); + assert_eq!(undo_labels(&s), ["Enter Cell", "Enter Cell"], "the undone step is back"); + assert_eq!(s.doc().unwrap().redo.len(), 0); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(v(&s, "A1").display(), "1"); } // ---------------------------------------------------------------- recording buffer