diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index 74a70fd28..88d4faad2 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -932,16 +932,13 @@ fn use_item(s: &mut Session, p: &Value) -> Result { } } -/// Run `f`'s commands as one undo step (undone together when it fails), unless an undo group is -/// already open (its own step then holds them). +/// Run `f`'s commands as one undo step, or as part of the step of an undo group open in the active +/// document, and undo them together when it fails ([`Session::begin_undo_scope`]). fn in_one_step(s: &mut Session, f: impl FnOnce(&mut Session) -> Result) -> Result { - let open = s.active().is_some_and(|st| st.undo_group.is_some()); - if !open { - s.begin_undo_group(); - } + let scope = s.begin_undo_scope(); let r = f(s); - if !open { - s.end_undo_group(r.is_err()); + if let Some(scope) = scope { + s.end_undo_scope(scope, r.is_err()); } r } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 34a9c86da..535818524 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -305,6 +305,19 @@ pub struct UndoGroup { label: Option, } +/// One undo step begun with [`Session::begin_undo_scope`], which [`Session::end_undo_scope`] ends. +#[derive(Clone, Debug)] +pub struct UndoScope { + /// The document ([`DocState::uid`]). + uid: u64, + /// The journal's length when the scope began. + journal: usize, + /// When the document's undo group was open already: the document, its selection and the document + /// that the group's undo step keeps, as they were when the scope began; `None` when the scope + /// opened the group. + joined: Option<(Arc, Selection, Option>)>, +} + /// Per-document editing state. #[derive(Clone, Debug)] pub struct DocState { @@ -2054,6 +2067,72 @@ impl Session { self.journal.remove(&gone); } + /// [`Session::end_undo_group`] for the open document with [`DocState::uid`] `uid` only (it may + /// have closed since): its open undo group closes, and undo groups open in other documents stay + /// open. + fn end_document_undo_group(&mut self, uid: u64, cancel: bool) { + // The other documents' groups are set aside while `end_undo_group` closes this one's. + let others: Vec<(u64, UndoGroup)> = + self.docs.iter_mut().filter(|st| st.uid != uid).filter_map(|st| Some((st.uid, st.undo_group.take()?))).collect(); + self.end_undo_group(cancel); + for (other, g) in others { + if let Some(st) = self.document_mut(other) { + st.undo_group = Some(g); + } + } + } + + /// Make the edits that follow in the active document one undo step, until + /// [`Session::end_undo_scope`]: open an undo group there or, when one is open already, join it, + /// so that the edits are part of its step. `None` without a document. + pub fn begin_undo_scope(&mut self) -> Option { + let journal = self.journal.len(); + let st = self.active()?; + let uid = st.uid; + let joined = st.undo_group.as_ref().map(|g| (st.doc.clone(), st.selection.clone(), g.first.clone())); + if joined.is_none() { + self.begin_undo_group(); + } + Some(UndoScope { uid, journal, joined }) + } + + /// End `scope` in its own document, whichever document is active now; undo groups open in other + /// documents stay open. The undo group the scope opened closes as [`Session::end_undo_group`] + /// closes it. A scope that joined an open group leaves the group open; with `cancel`, it puts the + /// document and its selection back as they were when the scope began and cuts the journal back to + /// where the scope began (only while the group's step is still the newest one), and an undo step + /// that the scope's first edit recorded goes too. + pub fn end_undo_scope(&mut self, scope: UndoScope, cancel: bool) { + let Some((doc, selection, first)) = scope.joined else { + self.end_document_undo_group(scope.uid, cancel); + return; + }; + let guides_locked = self.menu.guides_locked; + let Some(st) = self.document_mut(scope.uid) else { return }; + let Some(g) = &mut st.undo_group else { return }; + let newest = g.first.as_ref().is_some_and(|f| st.history.undo.last().is_some_and(|e| Arc::ptr_eq(&e.doc, f))); + if !cancel || !newest { + return; + } + match &first { + // The scope's first edit recorded the group's step. + None => { + st.history.undo.pop(); + g.first = None; + } + Some(f) if g.first.as_ref().is_some_and(|now| Arc::ptr_eq(now, f)) => {} + Some(_) => return, + } + st.doc = doc; + st.selection = selection; + if guides_locked { + deselect_guides(&st.doc, &mut st.selection); + } + st.history.recost_last(&st.doc); + st.revision += 1; + self.journal.truncate(scope.journal); + } + /// Commands with enablement (for menus, palette, MCP `list_commands`). pub fn commands(&self) -> Vec { command_specs().iter().map(|c| c.info(self)).collect() diff --git a/crates/engine/src/tests_journal.rs b/crates/engine/src/tests_journal.rs index 713ad4b78..04d2bb71d 100644 --- a/crates/engine/src/tests_journal.rs +++ b/crates/engine/src/tests_journal.rs @@ -520,6 +520,80 @@ fn an_undo_group_is_one_undo_step() { assert_eq!(undo_len(&s), undo + 2); } +/// An undo scope ([`Session::begin_undo_scope`]) in a document without an open undo group opens +/// one, and [`Session::end_undo_scope`] closes that group only, whichever document is active, and +/// leaves another document's group open; canceled, it undoes only that group's edits and cuts the +/// journal back to where the group began. +#[test] +fn an_undo_scope_closes_only_its_own_documents_group() { + let mut s = Session::new(); + let rect = json!({"x": 10, "y": 10, "width": 100, "height": 50}); + let objects = |d: &DocState| d.doc.layers[0].children().map_or(0, |c| c.len()); + s.execute("file.new", &json!({"width": 400, "height": 300})).unwrap(); + s.begin_undo_group(); + s.execute("shape.rectangle", &rect).unwrap(); + s.execute("file.new", &json!({"width": 400, "height": 300})).unwrap(); + let journal = s.journal.len(); + let scope = s.begin_undo_scope().unwrap(); + assert!(s.doc().unwrap().undo_group.is_some(), "the scope opens a group"); + s.execute("shape.rectangle", &rect).unwrap(); + s.execute("shape.rectangle", &rect).unwrap(); + s.end_undo_scope(scope, true); + assert_eq!(objects(s.doc().unwrap()), 0, "canceled: the document's edits are undone"); + assert_eq!(s.journal.len(), journal, "and dropped from the journal"); + assert!(s.documents()[0].undo_group.is_some(), "the other document's group stays open"); + assert_eq!(objects(&s.documents()[0]), 1, "with its edit"); + + let undo = s.doc().unwrap().history.undo.len(); + let scope = s.begin_undo_scope().unwrap(); + s.execute("shape.rectangle", &rect).unwrap(); + s.execute("shape.rectangle", &rect).unwrap(); + s.set_active(0); + s.end_undo_scope(scope, false); + let second = &s.documents()[1]; + assert_eq!(second.history.undo.len(), undo + 1, "closed: one undo step"); + assert!(second.undo_group.is_none(), "closed while another document is active"); + assert!(s.documents()[0].undo_group.is_some(), "the active document's group stays open"); +} + +/// An undo scope begun while the document's undo group is open joins the group: ended, its edits +/// are part of the group's step and the group stays open; canceled, the document goes back to +/// where it stood when the scope began, with the edits made in the group before it, the journal +/// drops the scope's entries, and a step that the scope's first edit recorded goes too. +#[test] +fn an_undo_scope_in_an_open_group_takes_back_only_its_own_edits() { + let mut s = Session::new(); + let rect = json!({"x": 10, "y": 10, "width": 100, "height": 50}); + let objects = |s: &Session| s.doc().unwrap().doc.layers[0].children().map_or(0, |c| c.len()); + let undo_len = |s: &Session| s.doc().unwrap().history.undo.len(); + s.execute("file.new", &json!({"width": 400, "height": 300})).unwrap(); + let undo = undo_len(&s); + s.begin_undo_group(); + let journal = s.journal.len(); + let scope = s.begin_undo_scope().unwrap(); + s.execute("shape.rectangle", &rect).unwrap(); + s.end_undo_scope(scope, true); + assert_eq!((objects(&s), undo_len(&s), s.journal.len()), (0, undo, journal), "the group had no step: none is left"); + assert!(s.doc().unwrap().undo_group.is_some(), "the group stays open"); + + s.execute("shape.rectangle", &rect).unwrap(); + let journal = s.journal.len(); + let scope = s.begin_undo_scope().unwrap(); + s.execute("shape.rectangle", &rect).unwrap(); + s.execute("shape.rectangle", &rect).unwrap(); + s.end_undo_scope(scope, true); + assert_eq!((objects(&s), undo_len(&s), s.journal.len()), (1, undo + 1, journal), "the group's own edit stays"); + + let scope = s.begin_undo_scope().unwrap(); + s.execute("shape.rectangle", &rect).unwrap(); + s.end_undo_scope(scope, false); + assert_eq!((objects(&s), undo_len(&s)), (2, undo + 1), "ended: the edit joins the group's step"); + assert!(s.doc().unwrap().undo_group.is_some(), "the group stays open"); + s.end_undo_group(false); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(objects(&s), 0, "one undo takes back the group's edits"); +} + /// A move drag is named for what its last preview did: Alt pressed or released during the drag /// turns it into a copy or back (the Selection and Direct Selection tools). #[test] diff --git a/crates/engine/src/tests_library.rs b/crates/engine/src/tests_library.rs index 15c8d9be8..31398944c 100644 --- a/crates/engine/src/tests_library.rs +++ b/crates/engine/src/tests_library.rs @@ -143,6 +143,66 @@ fn text_styles_come_from_the_selected_type_and_apply_in_one_step() { assert_eq!(r["style"], format!("{name} 2")); } +/// A library with the character and paragraph styles of 31 pt type, an undo group opened in the +/// first document with a rectangle drawn in it, and a second document, active, with type +/// selected → (session, library, character style, paragraph style). +fn group_open_in_another_document() -> (Session, String, String, String) { + let mut s = session(); + let lib = run(&mut s, "library.create", json!({}))["id"].as_str().unwrap().to_string(); + let t = run(&mut s, "text.create", json!({"x": 10, "y": 50, "text": "Heading", "size": 31}))["id"].clone(); + run(&mut s, "select.set", json!({"ids": [t]})); + let char_style = run(&mut s, "library.add", json!({"library": lib, "kind": "charStyle"}))["name"].as_str().unwrap().to_string(); + let para_style = run(&mut s, "library.add", json!({"library": lib, "kind": "paraStyle"}))["name"].as_str().unwrap().to_string(); + run(&mut s, "edit.undoGroup.begin", json!({})); + run(&mut s, "shape.rectangle", json!({"x": 0, "y": 0, "width": 20, "height": 20})); + run(&mut s, "file.new", json!({"width": 200, "height": 200})); + let u = run(&mut s, "text.create", json!({"x": 10, "y": 50, "text": "Body", "size": 9}))["id"].clone(); + run(&mut s, "select.set", json!({"ids": [u]})); + (s, lib, char_style, para_style) +} + +/// A library style used in one document does not close an undo group that is open in another +/// document, so that document's next edit still joins the group's undo step. +#[test] +fn a_library_style_leaves_another_documents_undo_group_open() { + let (mut s, lib, name, _) = group_open_in_another_document(); + run(&mut s, "library.use", json!({"library": lib, "kind": "charStyle", "item": name})); + assert!(s.documents()[0].undo_group.is_some(), "the first document's undo group stays open"); + s.set_active(0); + let undo = s.doc().unwrap().history.undo.len(); + run(&mut s, "shape.rectangle", json!({"x": 30, "y": 0, "width": 20, "height": 20})); + assert_eq!(s.doc().unwrap().history.undo.len(), undo, "the edit joins the group's step"); +} + +/// A library style that fails to apply takes back only the edits it made in its own document: an +/// undo group open in another document stays open, and the edits made in it stay. +#[test] +fn a_library_style_that_fails_takes_back_only_its_own_edits() { + let (mut s, lib, _, name) = group_open_in_another_document(); + run(&mut s, "select.none", json!({})); + assert!(s.execute("library.use", &json!({"library": lib, "kind": "paraStyle", "item": name})).is_err(), "no type selected"); + assert!(s.doc().unwrap().doc.para_styles.iter().all(|p| p.name != name), "the style it added is taken back"); + let first = &s.documents()[0]; + assert_eq!(first.doc.layers[0].children().map_or(0, |c| c.len()), 2, "the type and the rectangle drawn in the group stay"); + assert!(first.undo_group.is_some(), "the first document's undo group stays open"); +} + +/// A library style that fails to apply inside an undo group open in its own document takes back +/// the style it added, and the group stays open with the edits made in it before. +#[test] +fn a_library_style_that_fails_in_an_open_group_takes_back_its_own_edits() { + let (mut s, lib, _, name) = group_open_in_another_document(); + s.set_active(0); + run(&mut s, "select.none", json!({})); + let undo = s.doc().unwrap().history.undo.len(); + assert!(s.execute("library.use", &json!({"library": lib, "kind": "paraStyle", "item": name})).is_err(), "no type selected"); + let d = s.doc().unwrap(); + assert!(d.doc.para_styles.iter().all(|p| p.name != name), "the style it added is taken back"); + assert_eq!(d.doc.layers[0].children().map_or(0, |c| c.len()), 2, "the type and the rectangle drawn in the group stay"); + assert_eq!(d.history.undo.len(), undo, "still the group's one step"); + assert!(d.undo_group.is_some(), "the group stays open"); +} + /// A text style that a library file names with spaces around it comes into the document with the /// name trimmed, as New Character Style and New Paragraph Style trim a name, and applies. #[test] diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index e45553480..4a665906a 100644 --- a/crates/ui-egui/src/lib.rs +++ b/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, /// 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, } diff --git a/crates/ui-egui/src/scrub.rs b/crates/ui-egui/src/scrub.rs index 58d3405f7..91b7f10c1 100644 --- a/crates/ui-egui/src/scrub.rs +++ b/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(); } diff --git a/crates/ui-egui/src/tests_scrub.rs b/crates/ui-egui/src/tests_scrub.rs index fa00aeee4..adb6b161d 100644 --- a/crates/ui-egui/src/tests_scrub.rs +++ b/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"); +}