diff --git a/crates/engine/src/history.rs b/crates/engine/src/history.rs index ee6adccc4..4ce4e4599 100644 --- a/crates/engine/src/history.rs +++ b/crates/engine/src/history.rs @@ -129,8 +129,8 @@ impl History { } /// Which step the document is at: the newest undo step and the next redo step, by numbers no - /// other step takes (0 for none). Every new step, Undo and Redo changes it; an edit merged into - /// the newest step (an undo group) and a selection change do not. + /// other step takes (0 for none). Every new step, Undo, Redo and edit merged into the newest + /// step (an undo group) changes it; a selection change does not. pub fn step(&self) -> (u64, u64) { (self.undo.last().map_or(0, |e| e.serial), self.redo.last().map_or(0, |e| e.serial)) } @@ -161,11 +161,12 @@ impl History { } /// Estimate the newest undo step again, its next state now being `now` (the steps after it were - /// merged into it, or an undo group extended it). + /// merged into it, or an undo group extended it). It takes a new number ([`History::step`]). pub(crate) fn recost_last(&mut self, now: &Document) { if let Some(e) = self.undo.last_mut() { (e.bytes, e.next_bytes) = e.doc.retained_bytes_each(now); e.next = std::ptr::from_ref(now) as usize; + e.serial = next_serial(); } } } diff --git a/crates/engine/src/tests/blendopts.rs b/crates/engine/src/tests/blendopts.rs index 366a19241..059357ddb 100644 --- a/crates/engine/src/tests/blendopts.rs +++ b/crates/engine/src/tests/blendopts.rs @@ -235,7 +235,7 @@ fn escape_drops_the_blend_tools_first_pick() { #[test] fn an_edit_undo_or_redo_made_outside_the_blend_tool_drops_its_first_pick() { - for case in ["edit", "undo", "redo"] { + for case in ["edit", "undo", "redo", "group"] { let undo = case == "undo"; let mut s = session(); let (a, b, c) = (rect(&mut s, 0.0, 0.0, 40.0), rect(&mut s, 200.0, 0.0, 40.0), rect(&mut s, 400.0, 0.0, 40.0)); @@ -243,6 +243,11 @@ fn an_edit_undo_or_redo_made_outside_the_blend_tool_drops_its_first_pick() { s.execute("paint.setFill", &json!({"ids": [c.0], "color": "#ff0000"})).unwrap(); s.execute("edit.undo", &json!({})).unwrap(); } + // In an open undo group, the edit after the pick is merged into the group's step. + if case == "group" { + s.begin_undo_group(); + s.execute("paint.setFill", &json!({"ids": [c.0], "color": "#00ff00"})).unwrap(); + } s.select_tool("blend", ViewInfo::default()).unwrap(); let click = |s: &mut Session, x: f64, y: f64| s.pointer(&PointerEvent::new(PointerKind::Down, x, y), ViewInfo::default()).unwrap(); // Undo takes away c, the object picked first; an edit or a redo recolors c, which is not picked. diff --git a/crates/engine/src/tests/history_mem.rs b/crates/engine/src/tests/history_mem.rs index 23665d9ec..c14ea4aca 100644 --- a/crates/engine/src/tests/history_mem.rs +++ b/crates/engine/src/tests/history_mem.rs @@ -181,8 +181,8 @@ fn redo_during_an_interaction_keeps_the_history_and_the_document_in_step() { } /// `History::step` changes with every new step, Undo and Redo, also when an undo and a new edit -/// leave as many steps as before, and stays the same for an edit merged into an undo group and for -/// a selection change. +/// leave as many steps as before, and with an edit merged into an undo group's step. It stays the +/// same for a selection change. #[test] fn the_history_step_changes_with_each_step_undo_and_redo() { let mut s = session_with_rects(2); @@ -210,9 +210,13 @@ fn the_history_step_changes_with_each_step_undo_and_redo() { s.execute("select.all", &json!({})).unwrap(); s.begin_undo_group(); s.execute("object.move", &json!({"dx": 1, "dy": 0})).unwrap(); - let grouped = step(&s); - s.execute("object.move", &json!({"dx": 1, "dy": 0})).unwrap(); - assert_eq!(step(&s), grouped, "the second move is merged into the first"); - s.end_undo_group(false); + let (grouped, depth) = (step(&s), s.doc().unwrap().history.undo.len()); assert_ne!(grouped, other); + s.execute("object.move", &json!({"dx": 1, "dy": 0})).unwrap(); + assert_eq!(s.doc().unwrap().history.undo.len(), depth, "the second move is merged into the first"); + let merged = step(&s); + assert!(merged != grouped && merged.1 == 0, "a merged edit moves the step: {merged:?}"); + s.execute("select.none", &json!({})).unwrap(); + assert_eq!(step(&s), merged); + s.end_undo_group(false); }