diff --git a/crates/engine/src/guard.rs b/crates/engine/src/guard.rs index 2a633ca6..870350c8 100644 --- a/crates/engine/src/guard.rs +++ b/crates/engine/src/guard.rs @@ -88,4 +88,32 @@ mod tests { // The session keeps working. s.execute("shape.insert", &json!({"preset": "ellipse", "rect": [36, 236, 200, 400]})).unwrap(); } + + #[test] + fn a_command_that_fails_after_editing_leaves_the_document_alone() { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + let a = s.execute("shape.insert", &json!({"preset": "rect", "rect": [36, 36, 100, 100]})).unwrap()["id"].as_u64().unwrap(); + let b = s.execute("shape.insert", &json!({"preset": "rect", "rect": [236, 36, 100, 100]})).unwrap()["id"].as_u64().unwrap(); + let before = s.active().unwrap().doc.clone(); + let undo_len = s.active().unwrap().history.undo.len(); + // `shape.connect` inserts the requested preset first, then rejects it because it is not a line. + let r = s.execute("shape.connect", &json!({"from": a, "to": b, "preset": "rect"})); + assert!(r.is_err()); + let st = s.active().unwrap(); + assert!(std::sync::Arc::ptr_eq(&st.doc, &before), "the stray shape is gone"); + assert_eq!(st.history.undo.len(), undo_len); + } + + #[test] + fn a_failed_picture_change_adds_no_media() { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + s.execute("shape.insert", &json!({"preset": "rect", "rect": [36, 36, 100, 100]})).unwrap(); + let before = s.active().unwrap().doc.clone(); + // `picture.change` stores the image first, then finds no shape with this id. + let r = s.execute("picture.change", &json!({"ids": [9999], "name": "a.png", "data": crate::cmd::base64_encode(b"not a picture")})); + assert!(r.is_err()); + assert!(std::sync::Arc::ptr_eq(&s.active().unwrap().doc, &before), "the stored image is gone"); + } } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 981fcf16..6bcde52d 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -452,7 +452,23 @@ impl Session { self.depth += 1; let r = (spec.run)(self, params); self.depth -= 1; - let r = r?; + let r = match r { + Ok(r) => r, + Err(e) => { + // A command made of several edits can fail after some of them went through; the + // outermost one puts the presentation back, so a failed command changes nothing. + if outer + && let (Some((uid, old, sel)), Some(st)) = (before, self.active_mut()) + && st.uid == uid + && !Arc::ptr_eq(&old, &st.doc) + { + st.doc = old; + st.selection = sel; + st.revision += 1; + } + return Err(e); + } + }; if let Some(st) = self.active_mut() { clamp_selection(st); }