Engine: a command that panics leaves the undo history as it found it

The panic guard restored the workbook and selection but not the history, so
a nested edit made before the panic left a no-op undo step and the redo
history it had cleared stayed lost. The guard now truncates the steps made
during the command and puts the redo history back. Regression test, and the
architecture note says what the guard restores.
This commit is contained in:
prxnvt
2026-10-23 20:08:40 -04:00
parent 6b7c48f119
commit a3b5f8e45c
3 changed files with 35 additions and 2 deletions
+11 -1
View File
@@ -532,6 +532,9 @@ impl Session {
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.
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()));
// 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()),
@@ -551,13 +554,20 @@ impl Session {
Err(p) => {
let msg =
p.downcast_ref::<&str>().map(|s| s.to_string()).or_else(|| p.downcast_ref::<String>().cloned()).unwrap_or_else(|| "panic".into());
// Restore the workbook as it was, and drop the undo groups the command opened.
// Restore the workbook and its history as they were, and drop the undo groups the
// command opened.
self.groups.retain(|g| g.id < first_group);
if let Some((uid, wb, sel)) = &before
&& let Some(d) = self.docs.iter_mut().find(|d| d.uid == *uid)
{
d.wb = wb.clone();
d.selection = sel.clone();
if let Some((_, len)) = start {
d.undo.truncate(len);
}
if let Some(redo) = redo_before {
d.redo = redo;
}
d.calc.rebuild(&d.wb);
}
if self.depth == 0 {
+22
View File
@@ -543,6 +543,28 @@ fn a_group_ended_at_the_history_limit_replaces_only_the_oldest_step() {
assert_undo_redo(&mut s, &[("F1", "one"), ("F2", "two")]);
}
#[test]
fn a_command_that_panics_leaves_the_history_as_it_found_it() {
// A command that edits through a nested command and then panics: the nested step is gone
// with the workbook it changed, and the redo history the nested step cleared comes back.
crate::cmd::register_undoable_test_command("test.editThenPanic", |s, _| {
put(s, "A1", "half done")?;
panic!("after an edit");
});
let mut s = s();
set(&mut s, "A1", "1");
set(&mut s, "A1", "2");
s.execute("edit.undo", json!({})).unwrap();
assert_eq!((undo_labels(&s).len(), s.doc().unwrap().redo.len()), (1, 1));
let err = s.execute("test.editThenPanic", json!({})).unwrap_err().to_string();
assert!(err.contains("after an edit"), "{err}");
assert_eq!(v(&s, "A1").display(), "1", "the workbook is restored");
assert_eq!(undo_labels(&s), ["Enter Cell"], "no step for the nested edit");
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");
}
// ---------------------------------------------------------------- recording buffer
fn recorded_ids(s: &Session) -> Vec<String> {
+2 -1
View File
@@ -33,7 +33,8 @@ including tests, 2026-10-10.
`Session::execute(id, params)`. Commands are `CommandSpec { id, label, menu, shortcut, params,
enabled, run, journal, undoable }`; ids follow Excel's ribbon (`home.bold`, `insert.chart`,
`data.sortAscending`) plus primitives (`cell.set`, `selection.set`, `range.setValues`).
`Session::execute` catches escaped panics and restores the workbook (never-crash guard).
`Session::execute` catches escaped panics and restores the workbook, its selection and its
undo and redo history as they were before the command (never-crash guard).
2. **Edits** change the model through copy-on-write bands; undo keeps the previous snapshot.
Undo retains at most 100 steps and trims older steps when its estimated retained cell storage
exceeds 128 MiB. The newest step remains undoable even if that step alone exceeds the budget.