From 7497618f76e47f643a04096b413e10ace0105fc5 Mon Sep 17 00:00:00 2001 From: Valentin Klein Date: Sat, 10 Oct 2026 18:14:53 +0200 Subject: [PATCH] fix(engine): programmatic undo/redo ends a running command first (#178) * Programmatic undo/redo ends a running command first (#177) `undo` or `redo` run through engine.execute, MCP, the control channel or a script while an interactive command was still running (e.g. TEXT waiting for another line) swapped the drawing under that command. Interactive commands push their undo step only when they end, so `undo` popped the previous command's step instead: it removed the earlier geometry, dropped the running command's entities, and left the command running from a stale state. Both commands now first end the running command the way Esc does, exactly as Session::start already does for Cmd+Z, the menu and the command line. The command's work so far becomes one undo step, and `undo` then removes that step. Regression test: programmatic_undo_during_a_command_ends_it_first. Co-Authored-By: Claude Opus 5.5 * Move test away from other PRs' hunks to avoid merge conflicts Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- crates/engine/src/cmd/edit.rs | 11 +++++++++++ crates/engine/src/tests.rs | 22 ++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index aaf44a7..4b2dc2d 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -33,7 +33,17 @@ pub fn specs() -> Vec { ] } +/// A programmatic undo/redo (`engine.execute`, MCP, control channel, scripts) first ends a running +/// command the way Esc does, exactly as `Session::start` does for Cmd+Z, the menu or the command +/// line. Otherwise it would swap the drawing under the command and pop the previous command's step. +fn end_running_command(s: &mut Session) { + if s.running.is_some() { + s.cancel(); + } +} + fn run_undo(s: &mut Session, p: &Value) -> Result { + end_running_command(s); let n = p.get("count").and_then(Value::as_u64).unwrap_or(1).clamp(1, 1000); let mut labels = Vec::new(); for _ in 0..n { @@ -49,6 +59,7 @@ fn run_undo(s: &mut Session, p: &Value) -> Result { } fn run_redo(s: &mut Session, p: &Value) -> Result { + end_running_command(s); let n = p.get("count").and_then(Value::as_u64).unwrap_or(1).clamp(1, 1000); let mut labels = Vec::new(); for _ in 0..n { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 8296141..934954e 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -34,6 +34,28 @@ fn line_undo_option_and_aliases() { assert!(s.running.is_none()); } +#[test] +fn programmatic_undo_during_a_command_ends_it_first() { + let mut s = Session::new(); + s.execute("line", &json!({"points": [[0, 0], [10, 0]]})).unwrap(); + // TEXT stays active, waiting for the next line. + s.script("TEXT 0,0 5 0 HELLO\n").unwrap(); + assert!(s.running.is_some()); + assert_eq!(kinds(&s), vec!["Line", "Text"]); + // Like Ctrl+Z during a command: end it (as Esc does), then undo it, not the line. + s.execute("undo", &json!({})).unwrap(); + assert!(s.running.is_none()); + assert_eq!(kinds(&s), vec!["Line"]); + s.execute("redo", &json!({})).unwrap(); + assert_eq!(kinds(&s), vec!["Line", "Text"]); + // Redo while a command runs ends it the same way and leaves the drawing alone. + s.script("TEXT 0,-10 5 0\n").unwrap(); + assert!(s.running.is_some()); + s.execute("redo", &json!({})).unwrap(); + assert!(s.running.is_none()); + assert_eq!(kinds(&s), vec!["Line", "Text"]); +} + #[test] fn polar_and_direct_distance() { let mut s = Session::new();