diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index c1021bb4..b439766b 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -259,16 +259,23 @@ fn undo_group_end(s: &mut Session, p: &Value) -> Result { ok() } -fn undo(s: &mut Session, _: &Value) -> Result { - // What is still in progress (typing, a drag) is the step to undo: keep it first, so Undo takes - // back only that and Redo brings it back, instead of dropping it and undoing the step before. - // A typing session also ends in the Type tool, whose caret and marked text (IME) must follow - // the document. +/// What is still in progress (typing, a drag) is a step of its own: keep it before the history +/// moves, so Undo takes back only that and Redo brings it back, instead of dropping it and undoing +/// the step before. As any new edit it ends the steps to redo, so Redo never swaps the document +/// under an interaction that would then preview on, or cancel to, the document from before (#1719). +/// A typing session also ends in the Type tool, whose caret and marked text (IME) must follow the +/// document. +fn settle_interaction(s: &mut Session) -> Result<()> { let typing = typing_in_progress(s); s.commit_interaction()?; if typing { s.set_tool_option("endTyping", &Value::Bool(true)); } + Ok(()) +} + +fn undo(s: &mut Session, _: &Value) -> Result { + settle_interaction(s)?; let st = s.doc_mut()?; let e = st.history.undo.pop().ok_or_else(|| EngineError::Other("nothing to undo".into()))?; let label = e.label.clone(); @@ -286,6 +293,7 @@ fn undo(s: &mut Session, _: &Value) -> Result { } fn redo(s: &mut Session, _: &Value) -> Result { + settle_interaction(s)?; let st = s.doc_mut()?; let e = st.history.redo.pop().ok_or_else(|| EngineError::Other("nothing to redo".into()))?; let label = e.label.clone(); diff --git a/crates/engine/src/tests_history_mem.rs b/crates/engine/src/tests_history_mem.rs index 0ac0fab5..7728672c 100644 --- a/crates/engine/src/tests_history_mem.rs +++ b/crates/engine/src/tests_history_mem.rs @@ -153,3 +153,29 @@ fn undo_and_redo_reuse_the_estimates() { let delete = st.history.undo.last().unwrap(); assert_eq!(delete.bytes, fresh(delete, &st.doc)); } + +/// #1719: Redo while a drag is in progress keeps the drag as a step (which ends the steps to redo, +/// as any new edit does) instead of swapping the document under it: before, the next preview or a +/// cancel went back to the document from before the redo while the history said it was redone. +#[test] +fn redo_during_an_interaction_keeps_the_history_and_the_document_in_step() { + let mut s = session_with_rects(1); + s.execute("object.move", &json!({"dx": 10, "dy": 0})).unwrap(); + let moved = first_x(&s); + s.execute("edit.undo", &json!({})).unwrap(); + let start = first_x(&s); + // A drag that changed nothing yet is no step: Redo redoes, and a cancel has nothing to undo. + s.begin_interaction("Move").unwrap(); + s.execute("edit.redo", &json!({})).unwrap(); + s.cancel_interaction().unwrap(); + assert_eq!(first_x(&s), moved, "the redone move stays"); + s.execute("edit.undo", &json!({})).unwrap(); + // A drag that moved the art is the newest step: nothing is left to redo, and nothing is lost. + s.begin_interaction("Move").unwrap(); + s.preview("object.move", &json!({"dx": 3, "dy": 0})).unwrap(); + assert!(s.execute("edit.redo", &json!({})).is_err(), "the drag ended the steps to redo"); + assert!(!s.in_interaction()); + assert_eq!(first_x(&s), start + 3.0); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(first_x(&s), start); +}