mirror of
https://github.com/storytold/vectorcraft.git
synced 2026-11-01 13:47:21 -06:00
Engine guard: a recovered panic restores every document's undo history and the active document (#3599)
run_guarded put back only the active document's art, selection and interaction. An undo step pushed before the panic stayed (Undo then stepped to a state equal to the current one) and Redo was lost; other documents a command touched kept their half-applied edits; the active document was restored by index. It now snapshots every open document with its history and restores them, and the active document by uid. Fixes #3599
This commit is contained in:
@@ -199,27 +199,30 @@ impl Session {
|
||||
r
|
||||
}
|
||||
|
||||
/// A top-level command: a panic (a bug) becomes [`EngineError::Internal`] and the active
|
||||
/// document goes back to how it was before the command, instead of crashing the frontend.
|
||||
/// A top-level command: a panic (a bug) becomes [`EngineError::Internal`] and every open
|
||||
/// document goes back to how it was before the command (its art, selection, interaction and
|
||||
/// undo history), and the same document stays active, instead of crashing the frontend.
|
||||
pub(crate) fn run_guarded(&mut self, id: &str, f: impl FnOnce(&mut Self) -> Result<Value>) -> Result<Value> {
|
||||
let snapshot = self.active().map(|d| (d.uid, d.doc.clone(), d.selection.clone(), d.interaction.clone()));
|
||||
let active = self.active;
|
||||
let snapshot: Vec<_> =
|
||||
self.docs.iter().map(|d| (d.uid, d.doc.clone(), d.selection.clone(), d.interaction.clone(), d.history.clone())).collect();
|
||||
let active = self.active().map(|d| d.uid);
|
||||
let r = match guard::catch_panic(|| self.run_nested(f)) {
|
||||
Ok(r) => r,
|
||||
Err(msg) => {
|
||||
self.depth = 0;
|
||||
self.journal_note.clear();
|
||||
if let Some((uid, doc, selection, interaction)) = snapshot
|
||||
&& let Some(st) = self.docs.iter_mut().find(|d| d.uid == uid)
|
||||
{
|
||||
for (uid, doc, selection, interaction, history) in snapshot {
|
||||
// A document the command closed stays closed.
|
||||
let Some(st) = self.docs.iter_mut().find(|d| d.uid == uid) else { continue };
|
||||
st.doc = doc;
|
||||
st.selection = selection;
|
||||
st.interaction = interaction;
|
||||
st.history = history;
|
||||
cmd::globaledit::follow_history(st);
|
||||
st.revision += 1;
|
||||
}
|
||||
if active.is_some_and(|i| i < self.docs.len()) {
|
||||
self.active = active;
|
||||
if let Some(i) = active.and_then(|uid| self.docs.iter().position(|d| d.uid == uid)) {
|
||||
self.active = Some(i);
|
||||
}
|
||||
Err(EngineError::Internal { cmd: id.to_string(), msg })
|
||||
}
|
||||
|
||||
@@ -3521,6 +3521,46 @@ fn a_panicking_command_rolls_back_instead_of_crashing() {
|
||||
assert_eq!(s.journal.last().map(|j| j.0.as_str()), Some("shape.rectangle"));
|
||||
}
|
||||
|
||||
/// A command that recorded an undo step and then panicked leaves Undo and Redo as they were.
|
||||
#[test]
|
||||
fn a_panic_after_an_edit_leaves_undo_and_redo_as_they_were() {
|
||||
let mut s = session();
|
||||
let a = rect(&mut s, 10.0, 10.0, 100.0, 50.0);
|
||||
rect(&mut s, 200.0, 10.0, 50.0, 50.0);
|
||||
s.execute("edit.undo", &json!({})).unwrap();
|
||||
let (undo, redo) = (s.doc().unwrap().history.undo.len(), s.doc().unwrap().history.redo.len());
|
||||
assert_eq!((undo, redo), (1, 1));
|
||||
let r = s.run_guarded("test.panic", |s| {
|
||||
s.execute("edit.clear", &json!({"ids": [a.0]}))?;
|
||||
s.run_nested(|_| panic!("boom"))
|
||||
});
|
||||
assert!(matches!(r, Err(EngineError::Internal { .. })));
|
||||
let st = s.doc().unwrap();
|
||||
assert_eq!((st.history.undo.len(), st.history.redo.len()), (undo, redo));
|
||||
assert!(st.doc.node(a).is_some());
|
||||
}
|
||||
|
||||
/// A panic rolls back every document the command touched, and the active one stays the same.
|
||||
#[test]
|
||||
fn a_panic_rolls_back_other_documents_and_keeps_the_active_one() {
|
||||
let mut s = session();
|
||||
s.execute("file.new", &json!({"width": 400, "height": 400})).unwrap();
|
||||
let second = rect(&mut s, 10.0, 10.0, 20.0, 20.0);
|
||||
s.execute("document.activate", &json!({"index": 0})).unwrap();
|
||||
let first_uid = s.doc().unwrap().uid;
|
||||
let second_before = s.docs[1].doc.clone();
|
||||
let steps = s.docs[1].history.undo.len();
|
||||
let r = s.run_guarded("test.panic", |s| {
|
||||
s.docs[1].history.undo.clear();
|
||||
Arc::make_mut(&mut s.docs[1].doc).remove(second)?;
|
||||
s.run_nested(|_| panic!("boom"))
|
||||
});
|
||||
assert!(matches!(r, Err(EngineError::Internal { .. })));
|
||||
assert!(Arc::ptr_eq(&s.docs[1].doc, &second_before));
|
||||
assert_eq!(s.docs[1].history.undo.len(), steps);
|
||||
assert_eq!(s.doc().unwrap().uid, first_uid);
|
||||
}
|
||||
|
||||
/// New layers and sublayers share one "Layer N" numbering, whatever art the document holds, and
|
||||
/// never repeat a name.
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user