Actions: a recording keeps its steps when a canceled undo group drops earlier entries

When a cancel removes journal entries numbered below a recording's
start, the entries after them are numbered down. The recording's start
and the journal's hold kept the old number, so Stop Recording left out
as many of the recorded steps as there were entries removed, and the
hold no longer covered them. `Journal::remove` now moves the hold down
with the entries, and Stop Recording reads the start from the hold.

`Journal::replace` puts an entry in place of another and counts it at
its new size. A cancel uses it for the `document.activate` that takes
the place of a canceled group's entry that switched documents, so the
journal's size estimate does not keep the old entry's size.

crates/engine/src/journal.rs

@@ -105,16 +105,33 @@ impl Journal {
             !gone(i - 1)
         });
         self.bytes = self.sizes.iter().sum();
-        // Each entry that was let go already counts once.
-        let mut last = None;
-        let let_go = numbers.iter().take_while(|&&n| n < dropped).filter(|&&n| last.replace(n) != Some(n)).count();
-        self.dropped = dropped.saturating_sub(let_go);
+        // How many of the numbers are below `n`, each counted once.
+        let below = |n: usize| {
+            let mut last = None;
+            numbers.iter().take_while(|&&m| m < n).filter(|&&m| last.replace(m) != Some(m)).count()
+        };
+        self.held = self.held.map(|h| h.saturating_sub(below(h)));
+        self.dropped = dropped.saturating_sub(below(dropped));
+    }
+    /// Put `e` in place of entry number `i`, if kept.
+    pub fn replace(&mut self, i: usize, e: JournalEntry) {
+        let Some(k) = i.checked_sub(self.dropped) else { return };
+        if let (Some(old), Some(size)) = (self.entries.get_mut(k), self.sizes.get_mut(k)) {
+            *size = entry_bytes(&e);
+            *old = e;
+            self.bytes = self.sizes.iter().sum();
+        }
     }
     /// Keep every entry from number `from` on (a recording in progress), or stop (`None`).
     pub fn hold(&mut self, from: Option<usize>) {
         self.held = from;
         self.trim();
     }
+    /// The number the entries are kept from ([`Journal::hold`]). [`Journal::remove`] moves it down
+    /// with the entries it numbers down.
+    pub fn held(&self) -> Option<usize> {
+        self.held
+    }
 
     /// Let the oldest entries go while over the bounds (in batches: a quarter of the entry bound
     /// at a time, so a push doesn't shift the whole list every time).
@@ -223,12 +240,17 @@ mod tests {
             j.push(("object.move".into(), json!({"dx": i})));
         }
         let first = j.first_kept();
+        // A recording from entry 29,500 on: the entries removed before it move its start down.
+        j.hold(Some(29_500));
         // One entry that was let go already and two kept ones, two of them given twice.
         j.remove(&[first - 1, first - 1, 29_000, 29_000, 29_998]);
-        assert_eq!((j.len(), j.first_kept()), (29_997, first - 1));
+        assert_eq!((j.len(), j.first_kept(), j.held()), (29_997, first - 1, Some(29_498)));
         let dx = |j: &Journal, i: usize| j.get(i).map(|e| e.1["dx"].clone());
         assert_eq!((dx(&j, 28_998), dx(&j, 28_999)), (Some(json!(28_999)), Some(json!(29_001))));
         assert_eq!(j.last().map(|e| e.1["dx"].clone()), Some(json!(29_999)));
         assert_eq!(j.bytes, j.sizes.iter().sum::<usize>());
+        // A replaced entry is counted at its new size.
+        j.replace(29_000, big(0));
+        assert_eq!(j.bytes, j.iter().map(entry_bytes).sum::<usize>());
     }
 }

crates/engine/src/lib.rs

@@ -2046,10 +2046,8 @@ impl Session {
         // An entry that changed the active document becomes that change, so the entries after it run
         // in the same document when the journal is replayed.
         for &(n, index) in &journal {
-            if let Some(index) = index
-                && let Some(e) = self.journal.get_mut(n)
-            {
-                *e = ("document.activate".to_string(), serde_json::json!({ "index": index }));
+            if let Some(index) = index {
+                self.journal.replace(n, ("document.activate".to_string(), serde_json::json!({ "index": index })));
             }
         }
         let gone: Vec<usize> = journal.into_iter().filter(|(_, index)| index.is_none()).map(|(n, _)| n).collect();

crates/ui-egui/src/panels/actions.rs

@@ -118,6 +118,8 @@ pub fn show(app: &mut VectorcraftApp, ui: &mut egui::Ui) {
         if widgets::icon_button(ui, "square", tl!("Stop Playing/Recording"), false, 24.0).clicked()
             && let Some((set, name, start)) = app.ui.recording.take()
         {
+            // A canceled undo group numbers the steps down when it drops entries made before them.
+            let start = app.session.journal.held().unwrap_or(start);
             let steps = app.session.journal_for_action(start);
             app.session.journal.hold(None);
             if let Some(s) = app.ui.action_sets.get_mut(set) {
@@ -211,6 +213,37 @@ mod tests {
         assert_eq!(names(&app), before[1..], "nothing selected after a delete: nothing more goes");
     }
 
+    /// A recording keeps its steps when a canceled undo group drops journal entries made before the
+    /// recording began.
+    #[test]
+    fn a_recording_keeps_its_steps_when_a_canceled_undo_group_drops_earlier_entries() {
+        use super::super::tests_appearance::{click, frame_events};
+        let mut app = VectorcraftApp::new(Session::new(), Default::default());
+        let rect = |x: i32| json!({"x": x, "y": 10, "width": 20, "height": 20});
+        for (id, p) in [
+            ("file.new", json!({"width": 200, "height": 100})),
+            ("file.new", json!({"width": 200, "height": 100})),
+            ("document.activate", json!({"index": 0})),
+            ("edit.undoGroup.begin", json!({})),
+            ("shape.rectangle", rect(10)),
+        ] {
+            app.run(id, p).unwrap();
+        }
+        let ctx = egui::Context::default();
+        frame_events(&ctx, &mut app, vec![], show);
+        // The bottom bar's 24-point buttons, left to right: stop, record, play, delete.
+        let mut bar = crate::toolbar::tests::widget_rects(&ctx, egui::vec2(24.0, 24.0));
+        bar.sort_by(|a, b| a.left().total_cmp(&b.left()));
+        let [stop, record, _, _] = bar[..] else { panic!("the bottom bar: {bar:?}") };
+        click(&ctx, &mut app, record.center(), show);
+        for (id, p) in [("document.activate", json!({"index": 1})), ("shape.rectangle", rect(50)), ("edit.undoGroup.end", json!({"cancel": true}))] {
+            app.run(id, p).unwrap();
+        }
+        click(&ctx, &mut app, stop.center(), show);
+        let steps = app.ui.action_sets[0].actions.last().map(|a| a.steps.iter().map(|(c, _)| c.clone()).collect::<Vec<_>>());
+        assert_eq!(steps, Some(vec!["document.activate".to_string(), "shape.rectangle".into(), "edit.undoGroup.end".into()]));
+    }
+
     /// Actions recorded with Align to Artboard and Paste in Place, and the default Center on
     /// Artboard, play on the artboard active when they play, not on the one active when they were
     /// recorded.