Undo groups: library styles and field scrubs close only the group they open

Using a library's character or paragraph style, and scrubbing a
numeric field, closed their undo step with Session::end_undo_group,
which closes the open undo group of every document. An undo group open
in another document, such as one begun with edit.undoGroup.begin for
an MCP command_batch with atomic, was closed too, and that document's
next edits became undo steps of their own. When the style failed to
apply, or Escape ended the scrub, the other document's group was
canceled as well and the edits made in it were undone. Inside an undo
group already open in the same document, a scrub closed that group,
Escape undid the edits made in it before the drag, and a style that
failed to apply left the style it had added.

Session::begin_undo_scope and end_undo_scope make the edits in between
one undo step of the active document: the scope opens an undo group
there, or joins the one already open. Ending a scope closes only the
group it opened, in its own document, even when another document
became active in the meantime, and a canceled scope undoes only its
own edits, also inside a group that was already open. library.use and
the scrub use the scope, so Escape inside an open group also takes
back a scrub of the Transform panel's Shear field. edit.undoGroup.end
still closes the open group of every document.

crates/ui-egui/src/lib.rs

@@ -509,6 +509,8 @@ pub struct VectorcraftApp {
     /// A numeric field is being scrubbed: the document's edits meanwhile are one undo step
     /// ([`scrub::begin_frame`]).
     scrub_group: bool,
+    /// The undo step of the scrub in progress ([`vectorcraft_engine::Session::begin_undo_scope`]).
+    scrub_scope: Option<vectorcraft_engine::UndoScope>,
     /// The folders control requests, and the input they inject, may read and write
     /// (`--automation-read-root`, `--automation-write-root`); `None`: anywhere. See
     /// [`Self::with_automation_roots`].
@@ -610,6 +612,7 @@ impl VectorcraftApp {
             ime_discard: false,
             prefs_changed: false,
             scrub_group: false,
+            scrub_scope: None,
             automation_roots: None,
             synthetic_frame: false,
         }

crates/ui-egui/src/scrub.rs

@@ -80,23 +80,28 @@ pub(crate) fn phase(ctx: &Context) -> Phase {
     }
 }
 
-/// Before the panels draw: while a field is scrubbed, the edits its values make are one undo step.
-/// (The drag's first frame applies nothing, so the group is open before its first value.)
+/// Before the panels draw: while a field is scrubbed, the edits its values make are one undo step,
+/// or part of the step of an undo group open in the active document
+/// ([`vectorcraft_engine::Session::begin_undo_scope`]). (The drag's first frame applies nothing, so
+/// the step begins before its first value.)
 pub(crate) fn begin_frame(app: &mut VectorcraftApp, ctx: &Context) {
     if !app.scrub_group && matches!(phase(ctx), Phase::Dragging | Phase::Released) {
-        app.session.begin_undo_group();
+        app.scrub_scope = app.session.begin_undo_scope();
         app.scrub_group = true;
     }
 }
 
-/// After the panels: a finished drag closes its undo group; Escape undoes it.
+/// After the panels: a finished drag ends its undo step in the document it began in, even when
+/// another document is active by then; Escape undoes the drag's edits.
 pub(crate) fn end_frame(app: &mut VectorcraftApp, ctx: &Context) {
     let phase = phase(ctx);
     if !app.scrub_group || phase == Phase::Dragging {
         return;
     }
     app.scrub_group = false;
-    app.session.end_undo_group(phase == Phase::Cancelled);
+    if let Some(scope) = app.scrub_scope.take() {
+        app.session.end_undo_scope(scope, phase == Phase::Cancelled);
+    }
     app.sync_views();
 }
 

crates/ui-egui/src/tests_scrub.rs

@@ -6,7 +6,7 @@ use std::cell::Cell;
 use egui::{CursorIcon, Event, Key, Modifiers, PointerButton, Pos2, Rect, vec2};
 use serde_json::json;
 use vectorcraft_doc::Unit;
-use vectorcraft_engine::Session;
+use vectorcraft_engine::{DocState, Session};
 
 use crate::{VectorcraftApp, scrub, widgets};
 
@@ -239,6 +239,10 @@ fn undo_len(app: &VectorcraftApp) -> usize {
     app.session.doc().unwrap().history.undo.len()
 }
 
+fn objects(doc: &DocState) -> usize {
+    doc.doc.layers[0].children().map_or(0, |c| c.len())
+}
+
 /// Drag the W label by `dx` in four moves (`escape`: Escape before the release).
 fn drag_w(app: &mut VectorcraftApp, ctx: &egui::Context, dx: f32, escape: bool) {
     let mut w = None;
@@ -280,3 +284,78 @@ fn scrubbing_w_in_the_app_is_one_undo_step() {
     drag_w(&mut app, &ctx, 40.0, false);
     assert!((width(&app) - 100.0).abs() < 1e-6, "off: {}", width(&app));
 }
+
+/// A scrub does not close an undo group that is open in another document, and that document keeps
+/// its edits, when Escape ends the drag and when the drag is released.
+#[test]
+fn a_scrub_leaves_another_documents_undo_group_open() {
+    let mut app = VectorcraftApp::new(Session::new(), Default::default());
+    app.session.execute("file.new", &json!({"width": 400, "height": 300})).unwrap();
+    app.session.execute("edit.undoGroup.begin", &json!({})).unwrap();
+    app.session.execute("shape.rectangle", &json!({"x": 200, "y": 10, "width": 100, "height": 50})).unwrap();
+    app.session.execute("file.new", &json!({"width": 400, "height": 300})).unwrap();
+    app.session.execute("shape.rectangle", &json!({"x": 10, "y": 10, "width": 100, "height": 50})).unwrap();
+    let ctx = egui::Context::default();
+    let other = |app: &VectorcraftApp| (app.session.documents()[0].undo_group.is_some(), objects(&app.session.documents()[0]));
+    drag_w(&mut app, &ctx, 40.0, true);
+    assert!((width(&app) - 100.0).abs() < 1e-6, "Escape: {}", width(&app));
+    assert_eq!(other(&app), (true, 1), "Escape");
+    drag_w(&mut app, &ctx, 40.0, false);
+    assert!((width(&app) - (100.0 + 40.0 / scrub::PX_PER_STEP as f64)).abs() < 1e-6, "{}", width(&app));
+    assert_eq!(other(&app), (true, 1), "released");
+}
+
+/// A scrub in a document whose undo group is already open joins that group's undo step and
+/// leaves the group open; Escape puts the document back as it was before the drag, with the edits
+/// made in the group, and drops the drag's journal entries.
+#[test]
+fn a_scrub_inside_an_open_undo_group_joins_its_step() {
+    let (mut app, ctx) = app_with_rect();
+    app.run("edit.undoGroup.begin", json!({})).unwrap();
+    app.run("shape.rectangle", json!({"x": 200, "y": 10, "width": 100, "height": 50})).unwrap();
+    let undo = undo_len(&app);
+    let (before, journal) = (app.session.doc().unwrap().doc.clone(), app.session.journal.len());
+    drag_w(&mut app, &ctx, 40.0, true);
+    assert!((width(&app) - 100.0).abs() < 1e-6, "Escape: {}", width(&app));
+    assert!(std::sync::Arc::ptr_eq(&app.session.doc().unwrap().doc, &before), "Escape puts the document back");
+    assert_eq!(app.session.journal.len(), journal, "and drops the drag's journal entries");
+    assert_eq!(objects(app.session.doc().unwrap()), 2, "the rectangle drawn in the group stays");
+    drag_w(&mut app, &ctx, 40.0, false);
+    assert!((width(&app) - (100.0 + 40.0 / scrub::PX_PER_STEP as f64)).abs() < 1e-6, "{}", width(&app));
+    assert!(app.session.doc().unwrap().undo_group.is_some(), "the group stays open");
+    assert_eq!(undo_len(&app), undo, "the drag joins the group's step");
+    app.run("edit.undoGroup.end", json!({})).unwrap();
+    app.run("edit.undo", json!({})).unwrap();
+    assert_eq!(objects(app.session.doc().unwrap()), 1, "one undo takes back the rectangle and the drag");
+}
+
+/// A scrub closes the undo group it opened in its own document when another document becomes
+/// active during the drag (as `document.activate` does), and leaves that document's group open.
+#[test]
+fn a_scrub_closes_its_group_in_its_own_document_after_a_switch() {
+    let mut app = VectorcraftApp::new(Session::new(), Default::default());
+    app.session.execute("file.new", &json!({"width": 400, "height": 300})).unwrap();
+    app.session.execute("edit.undoGroup.begin", &json!({})).unwrap();
+    app.session.execute("file.new", &json!({"width": 400, "height": 300})).unwrap();
+    app.session.execute("shape.rectangle", &json!({"x": 10, "y": 10, "width": 100, "height": 50})).unwrap();
+    let ctx = egui::Context::default();
+    let undo = undo_len(&app);
+    let mut w = None;
+    for _ in 0..4 {
+        w = app_frame(&mut app, &ctx, vec![], "W:");
+    }
+    let at = w.expect("the W label is shown").center();
+    let button = |pos, pressed| Event::PointerButton { pos, button: PointerButton::Primary, pressed, modifiers: Modifiers::NONE };
+    app_frame(&mut app, &ctx, vec![Event::PointerMoved(at)], "");
+    app_frame(&mut app, &ctx, vec![button(at, true)], "");
+    for i in 1..=4 {
+        app_frame(&mut app, &ctx, vec![Event::PointerMoved(at + vec2(10.0 * i as f32, 0.0))], "");
+    }
+    app.session.set_active(0);
+    app_frame(&mut app, &ctx, vec![button(at + vec2(40.0, 0.0), false)], "");
+    app_frame(&mut app, &ctx, vec![], "");
+    let second = &app.session.documents()[1];
+    assert!(second.undo_group.is_none(), "the scrub's group is closed");
+    assert_eq!(second.history.undo.len(), undo + 1, "one undo step for the drag");
+    assert!(app.session.documents()[0].undo_group.is_some(), "the active document's group stays open");
+}