Merge pull request #1856 from storytold/fix/1719-redo-interaction

Fix #1719: Redo keeps a drag in progress as a step, as Undo does

crates/engine/src/cmd/edit.rs

@@ -259,16 +259,23 @@ fn undo_group_end(s: &mut Session, p: &Value) -> Result<Value> {
     ok()
 }
 
-fn undo(s: &mut Session, _: &Value) -> Result<Value> {
-    // 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<Value> {
+    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<Value> {
 }
 
 fn redo(s: &mut Session, _: &Value) -> Result<Value> {
+    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();

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);
+}