From 3d87b71ed43eba28f23068d3d03d0321c21352d1 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Mon, 5 Oct 2026 14:22:36 +0700 Subject: [PATCH] M14.4: a batch that opens a new document is journaled command.batch kept its steps in one interaction of the document active when it began and journaled itself by committing that interaction. A step that opened, closed or reverted a document broke both: the switch cancelled the interaction (throwing away the steps so far), and the commit then ran in the new document, which had none, so the batch was never journaled and a replay was missing the new document. An error after the switch rolled back only the active document. - Each step runs inside an interaction of the document active when it starts; a switch leaves it open while a batch runs, so every document the batch edits gets one undo step when it succeeds. - The batch journals itself, whichever document it ends in. - On error everything rolls back: documents the batch opened close, the ones it closed or reverted come back (Session::batch_stash), each one's interaction is undone, and the active document and the Untitled-N count go back too. - DocState::keep_interaction / undo_interaction share the undo-step and rollback code with commit_interaction / cancel_interaction. - Tests: a batch that opens a new document is journaled, replays to the same documents and keeps one undo step in each; a failed batch closes the document it opened and restores the one it closed; a failed batch brings back a document it reverted. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/fileio/batch.rs | 105 ++++++++++++++++++-------- crates/engine/src/cmd/fileio/mod.rs | 2 +- crates/engine/src/lib.rs | 69 +++++++++++------ crates/engine/src/tests_journal.rs | 78 +++++++++++++++++++ docs/mcp.md | 3 + 5 files changed, 202 insertions(+), 55 deletions(-) diff --git a/crates/engine/src/cmd/fileio/batch.rs b/crates/engine/src/cmd/fileio/batch.rs index a975d6c2..39a5026f 100644 --- a/crates/engine/src/cmd/fileio/batch.rs +++ b/crates/engine/src/cmd/fileio/batch.rs @@ -1,50 +1,89 @@ -//! `command.batch`: several commands as one undo step. +//! `command.batch`: several commands as one undo step (in each document they edit). use serde_json::{Value, json}; use super::super::*; -use crate::EngineError; +use crate::{DocState, EngineError}; pub(super) fn batch(s: &mut Session, p: &Value) -> Result { let mut cmds = p.get("commands").and_then(Value::as_array).ok_or_else(|| bad("command.batch", "missing commands"))?.clone(); let label = str_param(p, "label").unwrap_or("Batch").to_string(); - s.begin_interaction(&label)?; - // Session-level state a step may change (paint defaults, drawing mode) rolls back too. - let saved = (s.paint.clone(), s.fill_active, s.draw_mode, s.draw_inside, s.clipboard.clone()); - let mut results = vec![]; - for c in &mut cmds { - let id = c.get("command").and_then(Value::as_str).unwrap_or("").to_string(); - if id == "command.batch" { - s.cancel_interaction()?; - return Err(bad("command.batch", "batches cannot nest")); + // What an error rolls back besides the documents: session-level state a step may change + // (paint defaults, drawing mode, clipboard, the Untitled-N count). + let saved = (s.paint.clone(), s.fill_active, s.draw_mode, s.draw_inside, s.clipboard.clone(), s.untitled_counter); + let (start, active): (Vec, _) = (s.docs.iter().map(|d| d.uid).collect(), s.active); + s.batch_stash = Some(vec![]); + let r = run_steps(s, &label, &mut cmds); + let stash = s.batch_stash.take().unwrap_or_default(); + let results = match r { + Ok(results) => results, + Err(e) => { + roll_back(s, &start, stash); + s.active = active; + (s.paint, s.fill_active, s.draw_mode, s.draw_inside, s.clipboard, s.untitled_counter) = saved; + s.pending_paint = None; + return Err(e); } - let params = c.get("params").cloned().unwrap_or(json!({})); - match s.execute_step(&id, ¶ms) { - Ok((v, noted)) => { - // The step's journal entry records what it took from the preferences or the - // clock (a save's date), as a command of its own would. - if noted != params - && let Some(c) = c.as_object_mut() - { - c.insert("params".into(), noted); - } - results.push(v); - } - Err(e) => { - s.cancel_interaction()?; - (s.paint, s.fill_active, s.draw_mode, s.draw_inside, s.clipboard) = saved; - return Err(EngineError::Other(format!("batch step {} (`{id}`) failed: {e}", results.len()))); - } + }; + // Each document the steps edited keeps them as one undo step. + s.remember_pending_paint(); + for st in &mut s.docs { + if let Some(it) = st.interaction.take() { + st.keep_interaction(it); } } - // Commit as one undo step (the interaction's "preview" is the batch itself). + // The batch journals itself, whichever document it ends in. let mut entry = p.clone(); if let Some(o) = entry.as_object_mut() { o.insert("commands".into(), Value::Array(cmds)); } - if let Some(it) = s.doc_mut()?.interaction.as_mut() { - it.preview = Some(("command.batch".into(), entry)); - } - s.commit_interaction()?; + s.journal.push(("command.batch".into(), entry)); Ok(json!({ "results": results })) } + +/// Run the steps → their results. Each runs inside an interaction of the document active when it +/// starts (one for all the steps in that document), so a step may open, switch or close documents. +fn run_steps(s: &mut Session, label: &str, cmds: &mut [Value]) -> Result> { + let mut results = Vec::with_capacity(cmds.len()); + for c in cmds { + let id = c.get("command").and_then(Value::as_str).unwrap_or("").to_string(); + if id == "command.batch" { + return Err(bad("command.batch", "batches cannot nest")); + } + if s.active().is_some() { + s.begin_interaction(label)?; + } + let params = c.get("params").cloned().unwrap_or(json!({})); + let (v, noted) = + s.execute_step(&id, ¶ms).map_err(|e| EngineError::Other(format!("batch step {} (`{id}`) failed: {e}", results.len())))?; + // The step's journal entry records what it took from the preferences or the clock (a + // save's date, a new document's), as a command of its own would. + if noted != params + && let Some(c) = c.as_object_mut() + { + c.insert("params".into(), noted); + } + results.push(v); + } + Ok(results) +} + +/// The documents as they were before the batch (`start`: their uids in order): the ones it opened +/// close, the ones it closed or replaced (`stash`) come back, and each one's interaction is undone. +fn roll_back(s: &mut Session, start: &[u64], mut stash: Vec) { + let mut open = std::mem::take(&mut s.docs); + for &uid in start { + // The newest revision the document reached, so the restored one redraws. + let newest = open.iter().chain(&stash).filter(|d| d.uid == uid).map(|d| d.revision).max().unwrap_or_default(); + // Its state before the batch: the first one a step closed or replaced, else the open one. + let st = match stash.iter().position(|d| d.uid == uid) { + Some(i) => Some(stash.remove(i)), + None => open.iter().position(|d| d.uid == uid).map(|i| open.remove(i)), + }; + if let Some(mut st) = st { + st.undo_interaction(); + st.revision = newest.saturating_add(1); + s.docs.push(st); + } + } +} diff --git a/crates/engine/src/cmd/fileio/mod.rs b/crates/engine/src/cmd/fileio/mod.rs index 4a72a82a..482730c1 100644 --- a/crates/engine/src/cmd/fileio/mod.rs +++ b/crates/engine/src/cmd/fileio/mod.rs @@ -108,7 +108,7 @@ pub fn specs() -> Vec { has_doc, screens::export_for_screens ), - cmd!(query "command.batch", "Batch", [], None, "{label?, commands: [{command, params}]} run several commands as ONE undo step; stops at the first error and rolls back", has_doc, batch::batch), + cmd!(query "command.batch", "Batch", [], None, "{label?, commands: [{command, params}]} run several commands as ONE undo step (one in each document they edit: steps may open, switch, close or revert documents); stops at the first error and rolls everything back, documents included", has_doc, batch::batch), cmd!( query "document.formats", "File Formats", diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 0b4c64e7..2bb2d64a 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -168,6 +168,24 @@ impl DocState { pub fn mark_saved_as(&mut self, snapshot: &Arc) { self.saved_doc = snapshot.clone(); } + /// Keep what interaction `it` (taken from this document) changed, as one undo step. + pub(crate) fn keep_interaction(&mut self, it: Interaction) { + if !Arc::ptr_eq(&it.doc, &self.doc) { + self.history.undo.push(HistoryEntry { label: it.label, doc: it.doc, selection: it.selection }); + self.history.redo.clear(); + self.revision += 1; + } + } + /// End the interaction in progress, undoing what it changed. + pub(crate) fn undo_interaction(&mut self) { + if let Some(it) = self.interaction.take() { + self.doc = it.doc; + self.selection = it.selection; + self.active_layer = it.active_layer; + self.isolation = it.isolation; + self.revision += 1; + } + } pub fn title(&self) -> String { let name = self .path @@ -611,6 +629,9 @@ pub struct Session { /// The depth of the command whose values [`Session::note_journal`] keeps: 1 (the top-level /// command), or a step's while a batch runs it ([`Session::execute_step`]). note_depth: u32, + /// While `command.batch` runs: the documents its steps closed or replaced (Revert), which an + /// error brings back; `None` otherwise. + pub(crate) batch_stash: Option>, } impl Default for Session { @@ -649,6 +670,7 @@ impl Session { recent_urls: vec![], journal_note: Default::default(), note_depth: 1, + batch_stash: None, } } @@ -683,7 +705,11 @@ impl Session { let view = self.last_view; let acts = self.with_tool_cx(view, |t, cx| t.deactivate(cx)); let _ = self.apply_actions(acts); - let _ = self.cancel_interaction(); + // A batch leaves its interaction open in the document it leaves, to keep or roll back with + // the rest of the batch; anything else in progress (a drag) is cancelled. + if self.batch_stash.is_none() { + let _ = self.cancel_interaction(); + } self.tool = vectorcraft_tools::create(self.tool.id()); } pub fn set_active(&mut self, index: usize) -> bool { @@ -754,7 +780,10 @@ impl Session { st.save_options = old.save_options.clone(); st.converted = old.converted; st.view = old.view.clone(); - self.docs[index] = st; + let old = std::mem::replace(&mut self.docs[index], st); + if let Some(stash) = &mut self.batch_stash { + stash.push(old); + } true } pub fn close_document(&mut self, index: usize) -> bool { @@ -762,7 +791,10 @@ impl Session { return false; } self.reset_tool_for_doc_switch(); - self.docs.remove(index); + let old = self.docs.remove(index); + if let Some(stash) = &mut self.batch_stash { + stash.push(old); + } self.active = if self.docs.is_empty() { None } else { Some(index.min(self.docs.len() - 1)) }; true } @@ -778,6 +810,7 @@ impl Session { if self.depth == 0 { self.journal_note.clear(); self.note_depth = 1; + self.batch_stash = None; } let r = if self.depth == 0 { self.run_guarded(id, |s| (spec.run)(s, params)) } else { self.run_nested(|s| (spec.run)(s, params)) }; let r = r?; @@ -969,17 +1002,11 @@ impl Session { } pub fn commit_interaction(&mut self) -> Result<()> { - if let Some(p) = self.pending_paint.take() { - self.remember_paint_now(&p); - } + self.remember_pending_paint(); let st = self.doc_mut()?; - let Some(it) = st.interaction.take() else { return Ok(()) }; - let Some(preview) = it.preview else { return Ok(()) }; - if !Arc::ptr_eq(&it.doc, &st.doc) { - st.history.undo.push(HistoryEntry { label: it.label, doc: it.doc, selection: it.selection }); - st.history.redo.clear(); - st.revision += 1; - } + let Some(mut it) = st.interaction.take() else { return Ok(()) }; + let Some(preview) = it.preview.take() else { return Ok(()) }; + st.keep_interaction(it); if preview.0 == "object.transform" { let m = cmd::matrix_param(&preview.1, "matrix"); let copy = preview.1.get("copy").and_then(Value::as_bool).unwrap_or(false); @@ -991,16 +1018,16 @@ impl Session { Ok(()) } + /// Remember the paint a live preview applied (as its interaction is kept). + pub(crate) fn remember_pending_paint(&mut self) { + if let Some(p) = self.pending_paint.take() { + self.remember_paint_now(&p); + } + } + pub fn cancel_interaction(&mut self) -> Result<()> { self.pending_paint = None; - let st = self.doc_mut()?; - if let Some(it) = st.interaction.take() { - st.doc = it.doc; - st.selection = it.selection; - st.active_layer = it.active_layer; - st.isolation = it.isolation; - st.revision += 1; - } + self.doc_mut()?.undo_interaction(); Ok(()) } diff --git a/crates/engine/src/tests_journal.rs b/crates/engine/src/tests_journal.rs index a9e5d7d4..8bbaa605 100644 --- a/crates/engine/src/tests_journal.rs +++ b/crates/engine/src/tests_journal.rs @@ -153,3 +153,81 @@ fn a_batch_step_records_what_it_took_from_the_preferences() { let doc = |s: &Session| serde_json::to_value(&*s.doc().unwrap().doc).unwrap(); assert_eq!(doc(&r), doc(&s)); } + +/// Every open document of `s`, as JSON. +fn documents(s: &Session) -> Vec { + s.documents().iter().map(|d| serde_json::to_value(&*d.doc).unwrap()).collect() +} + +fn rect_step(x: i32) -> serde_json::Value { + json!({"command": "shape.rectangle", "params": {"x": x, "y": 10, "width": 20, "height": 20}}) +} + +#[test] +fn a_batch_that_opens_a_new_document_is_journaled_and_replays() { + let mut s = Session::new(); + s.execute("file.new", &json!({"created": null})).unwrap(); + let steps = json!([rect_step(10), {"command": "file.new", "params": {"width": 300, "height": 200}}, rect_step(50), rect_step(90)]); + s.execute("command.batch", &json!({"label": "Two documents", "commands": steps})).unwrap(); + // The steps before the new document stay in the first one and the rest go in the new one: one + // undo step in each, nothing left in progress. + assert_eq!(s.documents().len(), 2); + assert_eq!(s.active_index(), Some(1)); + for (st, objects) in s.documents().iter().zip([1, 2]) { + assert_eq!(st.doc.node_count(), 1 + objects); + assert_eq!(st.history.undo.iter().map(|h| h.label.as_str()).collect::>(), ["Two documents"]); + assert!(st.interaction.is_none()); + } + // The batch is journaled, with the new document's date, and replays to the same documents. + let (id, p) = s.journal.last().unwrap().clone(); + assert_eq!(id, "command.batch"); + assert!(p["commands"][1]["params"]["created"].is_i64(), "{p}"); + let r = replayed(&s); + assert_eq!(documents(&r), documents(&s)); + assert_eq!(r.active_index(), s.active_index()); + // Undo in the new document takes its part of the batch back. + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(s.doc().unwrap().doc.node_count(), 1); +} + +#[test] +fn a_failed_batch_closes_the_documents_it_opened_and_restores_the_ones_it_closed() { + let mut s = Session::new(); + s.execute("file.new", &json!({"created": null})).unwrap(); + s.execute("file.new", &json!({"created": null})).unwrap(); + s.execute("shape.ellipse", &json!({"x": 0, "y": 0, "width": 30, "height": 30})).unwrap(); + let (before, active, journal, next) = (documents(&s), s.active_index(), s.journal.len(), s.peek_untitled()); + let steps = json!([ + rect_step(10), + {"command": "file.close", "params": {}}, + rect_step(30), + {"command": "file.new", "params": {}}, + rect_step(50), + {"command": "no.such.command", "params": {}}, + ]); + let e = s.execute("command.batch", &json!({ "commands": steps })).unwrap_err(); + assert!(e.to_string().contains("batch step 5"), "{e}"); + assert_eq!(documents(&s), before); + assert_eq!(s.active_index(), active); + assert_eq!(s.journal.len(), journal, "a failed batch isn't journaled"); + assert_eq!(s.peek_untitled(), next, "the next new document keeps its name"); + assert!(s.documents().iter().all(|d| d.interaction.is_none())); + assert_eq!(s.documents()[1].history.undo.len(), 1, "only the ellipse drawn before the batch"); +} + +#[test] +fn a_failed_batch_brings_back_a_document_it_reverted() { + let path = vectorcraft_testkit::temp_dir("journal-batch").join("revert.vectorcraft").to_string_lossy().into_owned(); + let mut s = Session::new(); + s.execute("file.new", &json!({"created": null})).unwrap(); + s.execute("document.save", &json!({"path": path, "modified": null})).unwrap(); + s.execute("shape.ellipse", &json!({"x": 0, "y": 0, "width": 30, "height": 30})).unwrap(); + let (before, revision) = (documents(&s), s.doc().unwrap().revision); + let steps = json!([rect_step(10), {"command": "file.revert", "params": {}}, rect_step(30), {"command": "no.such.command"}]); + assert!(s.execute("command.batch", &json!({ "commands": steps })).is_err()); + assert_eq!(documents(&s), before, "the unsaved ellipse is back"); + let st = s.doc().unwrap(); + assert_eq!(st.history.undo.len(), 1); + assert!(st.is_dirty() && st.interaction.is_none()); + assert!(st.revision > revision, "the restored document redraws"); +} diff --git a/docs/mcp.md b/docs/mcp.md index 5cb808fa..d5d406af 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -981,6 +981,9 @@ all of it as Dublin Core. `file.new {created}` and a save to a file (`document.s record the date they used, so replaying the journal makes the same document whenever it runs (an action recorded in the Actions panel leaves the date out: playing it later dates the document then). As a step of `command.batch`, a command records the date it used (and any value it takes from the preferences) in its step of the batch's journal entry. +A batch may open, switch, close or revert documents: each document it edits gets one undo step, the batch is +journaled whichever document it ends in, and an error rolls every document back (the ones it opened close, the ones +it closed or reverted come back). ```json {"name":"run_command","arguments":{"command":"file.info","params":{"author":"Ada","keywords":"poster, fair","rating":4,"copyrightStatus":"copyrighted","copyrightNotice":"© 2026 Ada"}}}