From 0cffe2ca08323c355de92b701056790bd830ebc7 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 24 Oct 2026 13:19:40 -0700 Subject: [PATCH] Edit Points: a panic or another document leaves no stale point type A command that panicked after changing the point type Edit Points remembers left the new type in place, since the guard put back only the document and selection. The guard now puts the remembered type back too. Shape ids repeat across documents, so a change of active document forgets the remembered type. --- crates/engine/src/guard.rs | 16 ++++++++++++++++ crates/engine/src/lib.rs | 13 +++++++++++++ crates/engine/src/tests.rs | 11 +++++++++++ 3 files changed, 40 insertions(+) diff --git a/crates/engine/src/guard.rs b/crates/engine/src/guard.rs index 1c2ceac9..21cf1f8a 100644 --- a/crates/engine/src/guard.rs +++ b/crates/engine/src/guard.rs @@ -36,11 +36,13 @@ impl Session { /// Run command `id` (`f`) inside the guard. pub(crate) fn guarded(&mut self, id: &str, f: impl FnOnce(&mut Session) -> Result) -> Result { let before = self.active().map(|d| (d.uid, d.doc.clone(), d.selection.clone(), d.revision)); + let vertex_kind = self.tool.vertex_kind.clone(); let depth = self.depth; match catch_unwind(AssertUnwindSafe(|| f(self))) { Ok(r) => r, Err(payload) => { self.depth = depth; + self.tool.vertex_kind = vertex_kind; let msg = panic_message(payload.as_ref()); // Keep the document as it was before the command. if let (Some((uid, doc, selection, revision)), Some(st)) = (before, self.active_mut()) @@ -91,6 +93,20 @@ mod tests { s.execute("shape.insert", &json!({"preset": "ellipse", "rect": [36, 236, 200, 400]})).unwrap(); } + #[test] + fn a_panicking_command_keeps_the_remembered_point_type() { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + let kept = Some((deckcraft_model::ShapeId(1), 0, 0, "straight".to_string())); + s.tool.vertex_kind = kept.clone(); + let r = s.guarded("test.panic", |s| { + s.tool.vertex_kind = Some((deckcraft_model::ShapeId(1), 0, 0, "corner".to_string())); + panic!("boom"); + }); + assert!(r.is_err()); + assert_eq!(s.tool.vertex_kind, kept); + } + #[test] fn a_command_that_fails_after_editing_leaves_the_document_alone() { let mut s = Session::new(); diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 4b8128cc..25e4a5f4 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -561,16 +561,21 @@ impl Session { self.active_mut().ok_or(EngineError::NoDocument) } pub fn set_active(&mut self, i: usize) { + let was = self.active().map(|d| d.uid); if i < self.docs.len() { self.active = Some(i); } + self.forget_vertex_kind_unless(was); } pub fn add_document(&mut self, d: DocState) -> usize { + let was = self.active().map(|d| d.uid); self.docs.push(d); self.active = Some(self.docs.len() - 1); + self.forget_vertex_kind_unless(was); self.docs.len() - 1 } pub fn close_document(&mut self, i: usize) { + let was = self.active().map(|d| d.uid); if i < self.docs.len() { let d = self.docs.remove(i); if let Some(dir) = &self.recovery_dir { @@ -578,6 +583,14 @@ impl Session { } self.active = if self.docs.is_empty() { None } else { Some(i.min(self.docs.len() - 1)) }; } + self.forget_vertex_kind_unless(was); + } + /// The point type Edit Points remembers names a shape of the active document, and another + /// document can have a shape with the same id: a change of active document forgets it. + fn forget_vertex_kind_unless(&mut self, was: Option) { + if self.active().map(|d| d.uid) != was { + self.tool.vertex_kind = None; + } } pub fn next_untitled(&mut self) -> String { self.untitled += 1; diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index b3a13743..0382e5f9 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2324,6 +2324,17 @@ fn edit_points_remembered_point_type_follows_undo_redo_and_its_shape() { drag(&mut s, ho6, to6, Mods::default()); let (hi6, _, _) = handles_of(&s); assert!((hi6 - (v6 - Vec2::new(0.0, 40.0))).hypot() < 1e-3, "the other handle mirrors the dragged one: {hi6:?}"); + // Another document can have a shape with the same id: a change of active document forgets the + // remembered type, and choosing the active document again keeps it. + s.execute("shape.pointType", &json!({"index": 0, "type": "corner"})).unwrap(); + s.execute("file.new", &json!({})).unwrap(); + assert_eq!(s.tool.vertex_kind, None); + s.tool.vertex_kind = Some((other, 0, 0, "corner".to_string())); + let new_doc = s.active_index().unwrap(); + s.set_active(new_doc); + assert!(s.tool.vertex_kind.is_some()); + s.set_active(0); + assert_eq!(s.tool.vertex_kind, None); } #[test]