From c3af6605ef0fbac1598e49cadef6e4aae34fe891 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Tue, 27 Oct 2026 20:39:27 -0700 Subject: [PATCH 1/2] Type: Undo and Redo put the caret where the undone or redone edit was While the Type tool edits type, Edit > Undo and Redo (the menu, the keyboard, the History panel and MCP all run edit.undo and edit.redo) left the caret at its byte offset, clamped to the text. Typing "abc" before "XYZ" and pressing Undo left the caret after "XYZ". The engine now tells the active tool about edit.undo and edit.redo before it finishes the command. The Type tool keeps the edited text and a list of the steps that changed it, and puts the caret, with nothing selected, at the end of the span the step changed: where typed text began after its Undo, after restored text after the Undo of a deletion, and after retyped text after Redo. For the steps the tool made, it keeps where the span ended, so text that repeats the characters next to it puts the caret at the right end. A step that changes no characters, such as a formatting change, leaves the caret and the selection as they are. Typed lists keep their caret positions. --- crates/engine/src/session/execute.rs | 2 +- crates/engine/src/tests.rs | 1 + crates/engine/src/tests/typeundo.rs | 54 +++++++ crates/engine/src/tooling.rs | 9 +- crates/tools/src/text.rs | 133 ++++++++++++++++- crates/tools/src/text/tests.rs | 213 +++++++++++++++++++++++++++ crates/tools/src/text/tool.rs | 9 +- docs/mcp.md | 2 +- 8 files changed, 414 insertions(+), 9 deletions(-) create mode 100644 crates/engine/src/tests/typeundo.rs diff --git a/crates/engine/src/session/execute.rs b/crates/engine/src/session/execute.rs index bede24c7e..13e140d97 100644 --- a/crates/engine/src/session/execute.rs +++ b/crates/engine/src/session/execute.rs @@ -121,7 +121,7 @@ impl Session { } if self.depth == 0 { self.journal_in_undo_groups(doc, &grouped, journal); - self.after_command(); + self.after_command(Some(id)); } Ok(r) } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index bb115d752..8b2fe6aba 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -165,6 +165,7 @@ mod transparencygrid; mod trap; mod typearea; mod typescale; +mod typeundo; mod units; mod variables; mod webexport; diff --git a/crates/engine/src/tests/typeundo.rs b/crates/engine/src/tests/typeundo.rs new file mode 100644 index 000000000..2c7bf46d0 --- /dev/null +++ b/crates/engine/src/tests/typeundo.rs @@ -0,0 +1,54 @@ +//! Edit › Undo and Redo while the Type tool edits: the caret goes to where the step changed the text. + +use serde_json::json; +use vectorcraft_doc::{NodeId, NodeKind}; +use vectorcraft_tools::{Mods, PointerEvent, PointerKind, ToolKey}; + +use crate::*; + +fn session() -> Session { + let mut s = Session::new(); + s.prefs.placeholder_text = false; + s.execute("file.new", &json!({"width": 800, "height": 600})).unwrap(); + s +} + +fn plain(s: &Session, id: NodeId) -> String { + match &s.doc().unwrap().doc.node(id).unwrap().kind { + NodeKind::Text(t) => t.plain_text(), + _ => panic!("not text"), + } +} + +fn caret(s: &Session) -> (u64, u64) { + let o = s.tool_options(); + (o["caret"].as_u64().unwrap(), o["anchor"].as_u64().unwrap()) +} + +/// "abc" typed before "XYZ": Undo puts the caret before "XYZ" and Redo after "abc", from the +/// menu command and from the History panel's steps alike (both run `edit.undo` and `edit.redo`). +#[test] +fn undo_and_redo_put_the_caret_where_the_step_changed_the_text() { + let v = ViewInfo::default(); + let mut s = session(); + let r = s.execute("text.create", &json!({"x": 100, "y": 100, "text": "XYZ", "size": 20})).unwrap(); + let id = NodeId(r["id"].as_u64().unwrap()); + s.select_tool("type", v).unwrap(); + for k in [PointerKind::Down, PointerKind::Up] { + s.pointer(&PointerEvent::new(k, 101.0, 95.0), v).unwrap(); + } + s.set_tool_option("select", &json!({"start": 0, "end": 0})); + s.tool_text("abc", v).unwrap(); + assert_eq!((plain(&s, id).as_str(), caret(&s)), ("abcXYZ", (3, 3))); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!((plain(&s, id).as_str(), caret(&s)), ("XYZ", (0, 0))); + s.execute("edit.redo", &json!({})).unwrap(); + assert_eq!((plain(&s, id).as_str(), caret(&s)), ("abcXYZ", (3, 3))); + // A deleted character comes back with the caret after it, wherever the caret went since. + s.tool_key(ToolKey::End, Mods::default(), v).unwrap(); + s.tool_key(ToolKey::Backspace, Mods::default(), v).unwrap(); + assert_eq!(plain(&s, id), "abcXY"); + s.tool_key(ToolKey::Home, Mods::default(), v).unwrap(); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!((plain(&s, id).as_str(), caret(&s)), ("abcXYZ", (6, 6))); +} diff --git a/crates/engine/src/tooling.rs b/crates/engine/src/tooling.rs index a2e074377..62a559a67 100644 --- a/crates/engine/src/tooling.rs +++ b/crates/engine/src/tooling.rs @@ -181,7 +181,7 @@ impl Session { let r = self.apply_actions(acts); self.keep_tool_settings(); self.tool = lender; - self.after_command(); + self.after_command(None); r.map(drop) } @@ -678,10 +678,15 @@ impl Session { /// Let the active tool finish work a command from outside it ended ([`vectorcraft_tools::Tool::after_command`]): /// the Type tool and the Eyedropper clear a character range whose text is no longer selected. - pub(crate) fn after_command(&mut self) { + /// After `edit.undo` or `edit.redo` (`cmd`), the tool is told first, so the Type tool puts the + /// caret where the step changed the text. + pub(crate) fn after_command(&mut self, cmd: Option<&str>) { if self.in_tool_actions || (!self.tool.wants_text() && self.tool.id() != "eyedropper") { return; } + if let Some(cmd @ ("edit.undo" | "edit.redo")) = cmd { + self.with_tool_cx(self.last_view, |t, cx| t.notify(cx, cmd)); + } let acts = self.with_tool_cx(self.last_view, |t, cx| t.after_command(cx)); // The command itself succeeded: a failure here only leaves the edit as it was. if let Err(e) = self.apply_actions(acts) { diff --git a/crates/tools/src/text.rs b/crates/tools/src/text.rs index 087c9ea8d..85253daf0 100644 --- a/crates/tools/src/text.rs +++ b/crates/tools/src/text.rs @@ -87,6 +87,30 @@ struct TypedList { list: vectorcraft_doc::text::ParaList, } +/// An undo step that changed the text of the type object the tool edits, in the order the history +/// holds them. The plain text before and after it is kept as a hash. For a step the tool made (a +/// typing session or a typed list), `ends` holds the byte where the changed span ends before and +/// after it, so that Undo and Redo of the step put the caret there even where the change repeats +/// the characters next to it; a step a command from outside the tool made has none. +struct OwnEdit { + id: NodeId, + before: u64, + after: u64, + ends: Option<(usize, usize)>, + /// False once Undo took the step back, until Redo makes it again. + done: bool, +} + +/// How many undo steps [`TypeTool::edits`] keeps. +const OWN_EDITS_MAX: usize = 256; + +fn text_hash(s: &str) -> u64 { + use std::hash::{Hash, Hasher}; + let mut h = std::collections::hash_map::DefaultHasher::new(); + s.hash(&mut h); + h.finish() +} + /// The edited text's layout, keyed by the font database's generation and the text. type CachedLayout = ((u64, TextObject), Arc); @@ -152,6 +176,13 @@ pub struct TypeTool { /// The list typed characters last started (`text.startList`), so that after Undo puts them back /// the caret goes on after them, and after Redo it goes back. listed: Option, + /// The edited object's plain text as the tool last left or saw it, so that after Undo or Redo + /// the caret goes to the end of the span the step changed. + known: Option<(NodeId, String)>, + /// The tool's own last undo steps, oldest first. + edits: Vec, + /// The command that ran was Undo or Redo ([`TypeTool::after_history_step`]). + stepped: bool, cache: Mutex>, } @@ -321,7 +352,8 @@ impl TypeTool { self.typing = None; self.preedit = None; } - if self.typing.is_none() { + let fresh = self.typing.is_none(); + if fresh { out.push(Action::Begin("Typing".into())); self.typing_ended = false; } @@ -343,6 +375,7 @@ impl TypeTool { let runs = edit::slice_runs(&ty.cur, p, cur.len() - s); let runs = if runs.is_empty() { json!([]) } else { serde_json::to_value(&runs).unwrap_or(json!([])) }; out.push(Action::Preview("text.editRange".into(), json!({"id": id.0, "start": p, "end": base.len() - s, "runs": runs}))); + self.note_edit(fresh, id, (&base, base.len() - s), (&cur, cur.len() - s)); out } @@ -364,6 +397,7 @@ impl TypeTool { self.editing = Some(id); self.fresh = None; self.listed = None; + self.known = None; self.caret = caret; self.anchor = caret; self.goal_x = None; @@ -387,10 +421,100 @@ impl TypeTool { out.push(Action::Exec("text.startList".into(), json!({"id": id.0, "at": at}))); self.caret = start; self.anchor = start; - self.listed = Some(TypedList { id, start, end: at, text: t.plain_text(), list }); + let text = t.plain_text(); + if let Some(listed) = text.get(..start).zip(text.get(at..)).map(|(a, b)| format!("{a}{b}")) { + self.note_edit(true, id, (&text, at), (&listed, start)); + } + self.listed = Some(TypedList { id, start, end: at, text, list }); out } + /// Note the edited object's text as it is in the document now. + fn remember(&mut self, cx: &ToolContext) { + self.known = self.editing.and_then(|id| Some((id, Self::text(cx, id)?.plain_text()))); + } + + /// After a command from outside the tool: unless it was Undo or Redo, a command that changed + /// the edited text recorded a step, which drops the steps Undo took back from Redo, so the tool + /// drops them too and notes the step. A command that leaves the text as it was (a query) keeps + /// them. + fn after_outside_command(&mut self, cx: &ToolContext) { + let stepped = std::mem::take(&mut self.stepped); + let known = self.known.take(); + self.remember(cx); + if stepped || known == self.known { + return; + } + self.edits.retain(|e| e.done); + if let (Some((was, old)), Some((id, new))) = (&known, &self.known) + && was == id + { + let e = OwnEdit { id: *id, before: text_hash(old), after: text_hash(new), ends: None, done: true }; + self.push_edit(e); + } + } + + /// Note an undo step of the tool's own in `id`: the text before it and after it, each with the + /// byte where the changed span ends. `fresh` starts a new step; otherwise the step the tool + /// noted last grows (the typing session goes on). A new step drops the steps Undo took back, + /// as the history drops them from Redo. + fn note_edit(&mut self, fresh: bool, id: NodeId, before: (&str, usize), after: (&str, usize)) { + let e = OwnEdit { id, before: text_hash(before.0), after: text_hash(after.0), ends: Some((before.1, after.1)), done: true }; + self.edits.retain(|e| e.done); + match self.edits.last_mut() { + Some(last) if !fresh && last.id == id && last.ends.is_some() => *last = e, + _ => self.push_edit(e), + } + self.known = Some((id, after.0.to_string())); + } + + fn push_edit(&mut self, e: OwnEdit) { + if self.edits.len() >= OWN_EDITS_MAX { + self.edits.remove(0); + } + self.edits.push(e); + } + + /// After Undo or Redo, the caret goes to the end of the span the step changed in the edited + /// text, with nothing selected. A step that leaves the text as it was (a formatting change) + /// leaves the caret and the selection where they are, within the text. + fn after_history_step(&mut self, cx: &ToolContext, undo: bool) { + self.stepped = true; + let Some(id) = self.editing else { return }; + let Some(text) = Self::text(cx, id).map(TextObject::plain_text) else { return }; + match self.known.take() { + Some((known, old)) if known == id && old != text => { + let (old_hash, new_hash) = (text_hash(&old), text_hash(&text)); + // Undo takes back the object's last step still made that changed its text, and Redo + // makes again the one Undo took back last, when its texts match (a step that left + // the text as it was does not come here). A step the tool made knows where its + // span ends. Otherwise the text after the changed span is the common suffix (a + // removal's span is empty). + let changed = |e: &&mut OwnEdit| e.id == id && e.before != e.after; + let step = if undo { + self.edits.iter_mut().rev().filter(|e| e.done).find(changed).filter(|e| (e.after, e.before) == (old_hash, new_hash)) + } else { + self.edits.iter_mut().filter(|e| !e.done).find(changed).filter(|e| (e.before, e.after) == (old_hash, new_hash)) + }; + let own = step.and_then(|e| { + e.done = !undo; + e.ends.map(|(before, after)| if undo { before } else { after }) + }); + self.caret = match own { + Some(end) => edit::floor_char(&text, end), + None => text.len() - common_affixes(&old, &text, usize::MAX).1, + }; + self.anchor = self.caret; + self.goal_x = None; + } + _ => { + self.caret = edit::floor_char(&text, self.caret); + self.anchor = edit::floor_char(&text, self.anchor); + } + } + self.known = Some((id, text)); + } + /// After Undo of a list that typed characters started (and of what was typed after it), the /// text is as it was typed: the caret goes on after the characters. After Redo the text is as /// the list left it: the caret goes back to the paragraph's start. @@ -449,6 +573,7 @@ impl TypeTool { let byte = vectorcraft_text::hit_byte(&lay, t.xf.inverse() * p); let mut out = self.finish(cx); self.start_editing(h.leaf, byte); + self.remember(cx); self.clicks = (Some(p), 1); out.push(Action::Exec("select.set".into(), json!({"ids": [h.leaf.0]}))); Some(out) @@ -527,7 +652,8 @@ impl TypeTool { // new document text by a stale range. self.preedit = None; } - if self.typing.is_none() { + let fresh = self.typing.is_none(); + if fresh { out.push(Action::Begin("Typing".into())); self.typing_ended = false; } @@ -569,6 +695,7 @@ impl TypeTool { let runs = edit::slice_runs(&ty.cur, p, cur.len() - s); let runs = if runs.is_empty() { json!([]) } else { serde_json::to_value(&runs).unwrap_or(json!([])) }; out.push(Action::Preview("text.editRange".into(), json!({"id": id.0, "start": p, "end": base.len() - s, "runs": runs}))); + self.note_edit(fresh, id, (&base, base.len() - s), (&cur, cur.len() - s)); out } } diff --git a/crates/tools/src/text/tests.rs b/crates/tools/src/text/tests.rs index 75c1a4a7b..1b2086c30 100644 --- a/crates/tools/src/text/tests.rs +++ b/crates/tools/src/text/tests.rs @@ -262,6 +262,219 @@ fn typing_is_one_coalesced_preview() { assert!(a4.contains(&Action::Begin("Typing".into()))); } +/// What the engine does around Edit › Undo or Redo (`what`) that puts the document back to `to`: +/// it ends the typing session, runs the step, tells the tool, then lets it finish the command. +fn history_step(tool: &mut TypeTool, to: &Document, id: NodeId, what: &str) { + let (mut sel, p) = (Selection::default(), paint()); + sel.set([id]); + tool.set_option("endTyping", &json!(true)); + tool.notify(&cx(to, &sel, &p), what); + assert_eq!(tool.after_command(&cx(to, &sel, &p)), vec![]); +} + +fn type_in(tool: &mut TypeTool, d: &mut Document, s: &str) { + let (sel, p) = (Selection::default(), paint()); + let snap = d.clone(); + let acts = tool.text_input(&cx(d, &sel, &p), s); + apply(d, &snap, &acts); +} + +fn plain_of(d: &Document, id: NodeId) -> String { + let NodeKind::Text(t) = &d.node(id).unwrap().kind else { panic!("text") }; + t.plain_text() +} + +/// Undo of typing puts the caret where the typed text began, and Redo puts it after the text. +#[test] +fn undo_and_redo_of_typing_put_the_caret_where_the_text_changed() { + let (mut d, id, mut tool) = editing("XYZ"); + let before = d.clone(); + type_in(&mut tool, &mut d, "abc"); + assert_eq!((plain_of(&d, id).as_str(), tool.caret), ("abcXYZ", 3)); + let after = d.clone(); + history_step(&mut tool, &before, id, "edit.undo"); + assert_eq!((tool.caret, tool.anchor), (0, 0)); + history_step(&mut tool, &after, id, "edit.redo"); + assert_eq!((tool.caret, tool.anchor), (3, 3)); + // Typing in the middle of the text: Undo goes back to where the typing began. + tool.caret = 4; + tool.anchor = 4; + type_in(&mut tool, &mut d, "-"); + assert_eq!(plain_of(&d, id), "abcX-YZ"); + history_step(&mut tool, &after, id, "edit.undo"); + assert_eq!((tool.caret, tool.anchor), (4, 4)); +} + +/// Undo of a deletion puts the caret after the text it puts back, and Redo where it was taken out. +#[test] +fn undo_of_a_deletion_puts_the_caret_after_the_restored_text() { + let (mut d, id, mut tool) = editing("Hello big world"); + let before = d.clone(); + let snap = d.clone(); + tool.caret = 10; + tool.anchor = 10; + let acts = key(&mut tool, &d, ToolKey::Backspace, Mods { alt: true, ..Default::default() }); + apply(&mut d, &snap, &acts); + assert_eq!((plain_of(&d, id).as_str(), tool.caret), ("Hello world", 6)); + let after = d.clone(); + // The caret moved away before the Undo. + key(&mut tool, &d, ToolKey::End, Mods::default()); + history_step(&mut tool, &before, id, "edit.undo"); + assert_eq!((tool.caret, tool.anchor), (10, 10)); + history_step(&mut tool, &after, id, "edit.redo"); + assert_eq!((tool.caret, tool.anchor), (6, 6)); +} + +/// A step that changes no characters (a formatting change) leaves the caret and the selection, and +/// the Undo of the typing before it still puts the caret where the typing began. +#[test] +fn undo_of_formatting_leaves_the_caret_and_selection() { + let (mut d, id, mut tool) = editing("Hello world"); + let before = d.clone(); + tool.caret = 5; + tool.anchor = 5; + type_in(&mut tool, &mut d, ","); + let typed = d.clone(); + // A panel makes the typed text larger (a command from outside the tool). + let mut large = d.clone(); + let NodeKind::Text(t) = &mut large.node_mut(id).unwrap().kind else { panic!("text") }; + edit::style_range(&mut t.runs, 0, 6, |s| s.size = 30.0); + let (mut sel, p) = (Selection::default(), paint()); + sel.set([id]); + tool.set_option("endTyping", &json!(true)); + tool.after_command(&cx(&large, &sel, &p)); + tool.anchor = 2; + tool.caret = 8; + history_step(&mut tool, &typed, id, "edit.undo"); + assert_eq!((tool.anchor, tool.caret), (2, 8)); + history_step(&mut tool, &before, id, "edit.undo"); + assert_eq!((tool.anchor, tool.caret), (5, 5)); +} + +/// The caret lands on character boundaries in multi-byte text. +#[test] +fn undo_and_redo_keep_the_caret_on_character_boundaries() { + let (mut d, id, mut tool) = editing("café 🙂 ok"); + let before = d.clone(); + tool.caret = "café 🙂".len(); + tool.anchor = tool.caret; + type_in(&mut tool, &mut d, "é🙂"); + assert_eq!(plain_of(&d, id), "café 🙂é🙂 ok"); + let after = d.clone(); + history_step(&mut tool, &before, id, "edit.undo"); + assert_eq!(tool.caret, "café 🙂".len()); + history_step(&mut tool, &after, id, "edit.redo"); + assert_eq!(tool.caret, "café 🙂é🙂".len()); +} + +/// Typing or deleting the same characters as the text next to it: Undo and Redo put the caret where +/// the tool made the change, not at the other end of the repeated characters. +#[test] +fn undo_and_redo_of_repeated_characters_find_the_tools_own_edit() { + for (text, at, typed, undone, redone) in [("aaa", 0, "a", 0, 1), ("éé", 2, "é", 2, 4), ("ab ab", 3, "ab ", 3, 6)] { + let (mut d, id, mut tool) = editing(text); + let before = d.clone(); + tool.caret = at; + tool.anchor = at; + type_in(&mut tool, &mut d, typed); + let after = d.clone(); + history_step(&mut tool, &before, id, "edit.undo"); + assert_eq!((tool.caret, tool.anchor), (undone, undone), "{text}: undo"); + history_step(&mut tool, &after, id, "edit.redo"); + assert_eq!((tool.caret, tool.anchor), (redone, redone), "{text}: redo"); + } + // A deleted character next to the same character comes back with the caret after it. + let (mut d, id, mut tool) = editing("aaa"); + let before = d.clone(); + tool.caret = 1; + tool.anchor = 1; + let snap = d.clone(); + let acts = key(&mut tool, &d, ToolKey::Backspace, Mods::default()); + apply(&mut d, &snap, &acts); + assert_eq!(plain_of(&d, id), "aa"); + history_step(&mut tool, &before, id, "edit.undo"); + assert_eq!(tool.caret, 1); + // Two steps whose texts are each other's: each Undo and Redo finds its own step. + let (mut d, id, mut tool) = editing("aaa"); + let first = d.clone(); + type_in(&mut tool, &mut d, "a"); + let second = d.clone(); + key(&mut tool, &d, ToolKey::End, Mods::default()); + let snap = d.clone(); + let acts = key(&mut tool, &d, ToolKey::Backspace, Mods::default()); + apply(&mut d, &snap, &acts); + let third = d.clone(); + assert_eq!(plain_of(&third, id), "aaa"); + for (to, what, at) in [(&second, "edit.undo", 4), (&first, "edit.undo", 0), (&second, "edit.redo", 1), (&third, "edit.redo", 3)] { + history_step(&mut tool, to, id, what); + assert_eq!(tool.caret, at, "{what} to {}", plain_of(to, id)); + } + // A step from outside the tool after an Undo drops the undone step from Redo: Redo of the + // outside step does not take the dropped step's place. + let (mut d, id, mut tool) = editing("aaa"); + let first = d.clone(); + type_in(&mut tool, &mut d, "a"); + history_step(&mut tool, &first, id, "edit.undo"); + assert_eq!(tool.caret, 0); + let mut appended = first.clone(); + let NodeKind::Text(t) = &mut appended.node_mut(id).unwrap().kind else { panic!("text") }; + edit::replace_range(&mut t.runs, 3, 3, "a"); + let (mut sel, p) = (Selection::default(), paint()); + sel.set([id]); + assert_eq!(tool.after_command(&cx(&appended, &sel, &p)), vec![]); + history_step(&mut tool, &first, id, "edit.undo"); + history_step(&mut tool, &appended, id, "edit.redo"); + assert_eq!(tool.caret, 4); + // A command from outside the tool that leaves the text as it is (a query) keeps the undone + // step for Redo. + let (mut d, id, mut tool) = editing("aaa"); + let first = d.clone(); + type_in(&mut tool, &mut d, "a"); + let second = d.clone(); + history_step(&mut tool, &first, id, "edit.undo"); + let (mut sel, p) = (Selection::default(), paint()); + sel.set([id]); + assert_eq!(tool.after_command(&cx(&first, &sel, &p)), vec![]); + history_step(&mut tool, &second, id, "edit.redo"); + assert_eq!(tool.caret, 1); + // Commands from outside the tool that repeat the change of a step the tool made are steps of + // their own: Undo of the last one does not take back the tool's step. + let (mut d, id, mut tool) = editing("aaa"); + let first = d.clone(); + type_in(&mut tool, &mut d, "a"); + let typed = d.clone(); + let (mut sel, p) = (Selection::default(), paint()); + sel.set([id]); + tool.set_option("endTyping", &json!(true)); + let mut removed = typed.clone(); + let NodeKind::Text(t) = &mut removed.node_mut(id).unwrap().kind else { panic!("text") }; + edit::replace_range(&mut t.runs, 3, 4, ""); + assert_eq!(tool.after_command(&cx(&removed, &sel, &p)), vec![]); + let mut appended = removed.clone(); + let NodeKind::Text(t) = &mut appended.node_mut(id).unwrap().kind else { panic!("text") }; + edit::replace_range(&mut t.runs, 3, 3, "a"); + assert_eq!(tool.after_command(&cx(&appended, &sel, &p)), vec![]); + for (to, what, at) in [(&removed, "edit.undo", 3), (&typed, "edit.undo", 4), (&first, "edit.undo", 0), (&typed, "edit.redo", 1)] { + history_step(&mut tool, to, id, what); + assert_eq!(tool.caret, at, "{what} to {}", plain_of(to, id)); + } + // A typing session that leaves the text as it was (a character typed and deleted again) does + // not stop Undo of the step before it from finding where that step typed. + let (mut d, id, mut tool) = editing("aaa"); + let first = d.clone(); + type_in(&mut tool, &mut d, "a"); + key(&mut tool, &d, ToolKey::End, Mods::default()); + // The session's previews each replace the text it started from. + let session = d.clone(); + type_in(&mut tool, &mut d, "b"); + let acts = key(&mut tool, &d, ToolKey::Backspace, Mods::default()); + apply(&mut d, &session, &acts); + assert_eq!(plain_of(&d, id), "aaaa"); + history_step(&mut tool, &d, id, "edit.undo"); + history_step(&mut tool, &first, id, "edit.undo"); + assert_eq!(tool.caret, 0); +} + #[test] fn shift_arrows_select_and_typing_replaces() { let (d, _, mut tool) = editing("Hello world"); diff --git a/crates/tools/src/text/tool.rs b/crates/tools/src/text/tool.rs index 8845f551e..455a44b65 100644 --- a/crates/tools/src/text/tool.rs +++ b/crates/tools/src/text/tool.rs @@ -456,6 +456,7 @@ impl Tool for TypeTool { /// goes before the paragraph's text. fn after_command(&mut self, cx: &ToolContext) -> Vec { self.after_typed_list(cx); + self.after_outside_command(cx); let Some(id) = self.editing else { return vec![] }; let mut cur = Some(id).filter(|i| cx.doc.node(*i).is_some()); while let Some(c) = cur { @@ -466,12 +467,16 @@ impl Tool for TypeTool { } self.finish(cx) } - /// After `text.create` / `text.createInPath` the engine selects the new object; edit it. + /// After `text.create` / `text.createInPath` the engine selects the new object; edit it. After + /// `edit.undo` / `edit.redo` the caret goes to where the step changed the edited text. fn notify(&mut self, cx: &ToolContext, what: &str) { - if what == "text.editNew" + if matches!(what, "edit.undo" | "edit.redo") { + self.after_history_step(cx, what == "edit.undo"); + } else if what == "text.editNew" && let Some(id) = cx.selection.objects.first().copied() { self.start_editing(id, 0); + self.remember(cx); let t = Self::text(cx, id); self.fresh = t.filter(|t| matches!(t.kind, TextKind::Point)).map(|_| id); // Placeholder text comes selected: typing replaces it. diff --git a/docs/mcp.md b/docs/mcp.md index f4a62c3a7..744eead16 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -431,7 +431,7 @@ objects' fills or strokes differ (`fillMixed` / `strokeMixed`, drawn as a "?" pr | `transform` | `{ids?, dx?, dy?, rotate?, scale?, scaleX?, scaleY?, reflect?, shear?, origin?, copy?}` | Runs move, rotate, scale, reflect, shear in that order. With `copy`, the first step duplicates. | | `create_graph` | `{type?, x, y, width, height, csv? \| series?, categories?, rows?}` | The nine Illustrator graph types: column, stacked column, bar, stacked bar, line, area, scatter, pie and radar. An empty CSV cell or a `null` in `rows` is a blank value; a number in straight quotes is a label. Edit later with `graph.setData` / `graph.setType` via `run_command`. `graph.column {design, columnType: vertical\|uniform\|repeating\|sliding, each, fractions: chop\|scale}` draws the columns or bars of the series picked with Group Selection (or `seriesIndexes`, or all of them) with a graph design saved by `graph.design`; with no `design` it returns the series' current design and options. Type in the design that holds a `%` followed by two digits shows each column's value, with at least the first digit's places before the decimal point and the second digit's places after it. | | `text_wrap` | `{ids?, offset?, invert?, release?}` | Area type below the objects (same layer) flows around them. | -| `undo` / `redo` | `{}` | Many calls that change nothing record no undo step, for example `object.align` with nothing to move, `object.move` by 0 without `copy` and `object.showAll` with nothing hidden. `undo` (`edit.undo`) after such a call undoes the step before it. `inspect_document` lists the undo steps as `history`. `run_command` with `edit.clearHistory` removes every undo and redo step of the active document and returns `{cleared}`, the number of steps removed; the document stays as it is, and the command records no undo step. | +| `undo` / `redo` | `{}` | Many calls that change nothing record no undo step, for example `object.align` with nothing to move, `object.move` by 0 without `copy` and `object.showAll` with nothing hidden. `undo` (`edit.undo`) after such a call undoes the step before it. `inspect_document` lists the undo steps as `history`. `run_command` with `edit.clearHistory` removes every undo and redo step of the active document and returns `{cleared}`, the number of steps removed; the document stays as it is, and the command records no undo step. While the Type tool edits type, `undo` and `redo` put its caret, with no characters selected, at the end of the text the step changed: after the text it puts back or retypes, or where the text it takes out was. A step that changes no characters, such as a formatting change, leaves the caret and the selection where they are. | For example, create editable Japanese area type with the engine's vertical writing mode: From 43ecaaa62a988b9b40c081bbb09804c7d1948766 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 28 Oct 2026 01:51:12 -0700 Subject: [PATCH 2/2] Type: Undo and Redo in a command batch put the caret where the step was The Type tool heard of Undo and Redo only when they ran as top-level commands. As steps of command.batch, they left the caret where it was, and the tool took the batch for an outside edit. Session::execute now tells the active tool about each nested edit.undo and edit.redo. command.batch also tells the tool when it begins, ends or rolls back. The Type tool keeps its caret and its edit records at the start of each batch and puts them back when a failed batch rolls the documents back. --- crates/engine/src/cmd/fileio/batch.rs | 4 +++ crates/engine/src/session/execute.rs | 3 ++ crates/engine/src/tests/typeundo.rs | 26 ++++++++++++++++ crates/engine/src/tooling.rs | 13 +++++++- crates/tools/src/text.rs | 44 +++++++++++++++++++++++++++ crates/tools/src/text/tool.rs | 5 ++- 6 files changed, 93 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/fileio/batch.rs b/crates/engine/src/cmd/fileio/batch.rs index d5aa6d144..3dc3a51d9 100644 --- a/crates/engine/src/cmd/fileio/batch.rs +++ b/crates/engine/src/cmd/fileio/batch.rs @@ -27,15 +27,19 @@ pub(super) fn batch(s: &mut Session, p: &Value) -> Result { let active = s.active; s.batch_stash = Some(vec![]); let mut grouped = Vec::with_capacity(cmds.len()); + // The Type tool keeps what the steps' Undo and Redo change in it, to put back on a rollback. + s.notify_text_tool("batch.begin"); let r = run_steps(s, &label, artboard, &mut cmds, &mut grouped); let stash = s.batch_stash.take().unwrap_or_default(); let results = match r { Ok(results) => { + s.notify_text_tool("batch.end"); s.batch_steps = Some(grouped); results } Err(e) => { roll_back(s, start, &stash); + s.notify_text_tool("batch.rollBack"); s.active = active; (s.paint, s.fill_active, s.draw_mode, s.draw_inside, s.clipboard, s.untitled_counter, s.explicit_undo_group) = saved; s.pending_paint = None; diff --git a/crates/engine/src/session/execute.rs b/crates/engine/src/session/execute.rs index 13e140d97..24cd7096f 100644 --- a/crates/engine/src/session/execute.rs +++ b/crates/engine/src/session/execute.rs @@ -122,6 +122,9 @@ impl Session { if self.depth == 0 { self.journal_in_undo_groups(doc, &grouped, journal); self.after_command(Some(id)); + } else if matches!(id, "edit.undo" | "edit.redo") { + // A step of `command.batch`: the tool hears of it as it does of the command alone. + self.notify_text_tool(id); } Ok(r) } diff --git a/crates/engine/src/tests/typeundo.rs b/crates/engine/src/tests/typeundo.rs index 2c7bf46d0..66befa724 100644 --- a/crates/engine/src/tests/typeundo.rs +++ b/crates/engine/src/tests/typeundo.rs @@ -52,3 +52,29 @@ fn undo_and_redo_put_the_caret_where_the_step_changed_the_text() { s.execute("edit.undo", &json!({})).unwrap(); assert_eq!((plain(&s, id).as_str(), caret(&s)), ("abcXYZ", (6, 6))); } + +/// Undo and Redo run as steps of `command.batch` put the caret where they do run alone. +#[test] +fn undo_and_redo_in_a_batch_put_the_caret_where_the_step_changed_the_text() { + let v = ViewInfo::default(); + let mut s = session(); + let r = s.execute("text.create", &json!({"x": 100, "y": 100, "text": "XYZ", "size": 20})).unwrap(); + let id = NodeId(r["id"].as_u64().unwrap()); + s.select_tool("type", v).unwrap(); + for k in [PointerKind::Down, PointerKind::Up] { + s.pointer(&PointerEvent::new(k, 101.0, 95.0), v).unwrap(); + } + s.set_tool_option("select", &json!({"start": 0, "end": 0})); + s.tool_text("abc", v).unwrap(); + s.execute("command.batch", &json!({"commands": [{"command": "edit.undo"}]})).unwrap(); + assert_eq!((plain(&s, id).as_str(), caret(&s)), ("XYZ", (0, 0))); + s.execute("command.batch", &json!({"commands": [{"command": "edit.redo"}]})).unwrap(); + assert_eq!((plain(&s, id).as_str(), caret(&s)), ("abcXYZ", (3, 3))); + // A batch whose later step fails is rolled back: the text and the caret are as they were, and + // Undo still takes back the typing. + let failed = json!({"commands": [{"command": "edit.undo"}, {"command": "no.suchCommand"}]}); + assert!(s.execute("command.batch", &failed).is_err()); + assert_eq!((plain(&s, id).as_str(), caret(&s)), ("abcXYZ", (3, 3))); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!((plain(&s, id).as_str(), caret(&s)), ("XYZ", (0, 0))); +} diff --git a/crates/engine/src/tooling.rs b/crates/engine/src/tooling.rs index 62a559a67..82a538067 100644 --- a/crates/engine/src/tooling.rs +++ b/crates/engine/src/tooling.rs @@ -685,7 +685,7 @@ impl Session { return; } if let Some(cmd @ ("edit.undo" | "edit.redo")) = cmd { - self.with_tool_cx(self.last_view, |t, cx| t.notify(cx, cmd)); + self.notify_text_tool(cmd); } let acts = self.with_tool_cx(self.last_view, |t, cx| t.after_command(cx)); // The command itself succeeded: a failure here only leaves the edit as it was. @@ -694,6 +694,17 @@ impl Session { } } + /// Tell the active tool that `edit.undo` or `edit.redo` ran, on its own or as a step of + /// `command.batch`, so the Type tool puts the caret where the step changed the text; or that a + /// `command.batch` began, ended or was rolled back (`batch.begin`, `batch.end`, + /// `batch.rollBack`), so the Type tool puts back what its Undo and Redo steps changed when it fails. + pub(crate) fn notify_text_tool(&mut self, what: &str) { + if self.in_tool_actions || (!self.tool.wants_text() && self.tool.id() != "eyedropper") { + return; + } + self.with_tool_cx(self.last_view, |t, cx| t.notify(cx, what)); + } + /// Apply tool actions. Errors from previews are reported but keep the interaction alive. pub fn apply_actions(&mut self, acts: Vec) -> Result> { let outer = std::mem::replace(&mut self.in_tool_actions, true); diff --git a/crates/tools/src/text.rs b/crates/tools/src/text.rs index 85253daf0..2daa5a768 100644 --- a/crates/tools/src/text.rs +++ b/crates/tools/src/text.rs @@ -92,6 +92,7 @@ struct TypedList { /// typing session or a typed list), `ends` holds the byte where the changed span ends before and /// after it, so that Undo and Redo of the step put the caret there even where the change repeats /// the characters next to it; a step a command from outside the tool made has none. +#[derive(Clone)] struct OwnEdit { id: NodeId, before: u64, @@ -101,6 +102,18 @@ struct OwnEdit { done: bool, } +/// What Undo and Redo change in the tool, kept when a `command.batch` begins so that a batch that +/// fails and is rolled back puts it back ([`TypeTool::batch_notice`]). +struct HistoryState { + editing: Option, + caret: usize, + anchor: usize, + goal_x: Option, + known: Option<(NodeId, String)>, + edits: Vec, + stepped: bool, +} + /// How many undo steps [`TypeTool::edits`] keeps. const OWN_EDITS_MAX: usize = 256; @@ -183,6 +196,8 @@ pub struct TypeTool { edits: Vec, /// The command that ran was Undo or Redo ([`TypeTool::after_history_step`]). stepped: bool, + /// The state at the start of each `command.batch` still running, innermost last. + batches: Vec, cache: Mutex>, } @@ -515,6 +530,35 @@ impl TypeTool { self.known = Some((id, text)); } + /// A `command.batch` began (`batch.begin`), ended (`batch.end`), or failed and was rolled back + /// (`batch.rollBack`). After a rollback, what the batch's Undo and Redo steps changed in the tool + /// is as it was at the batch's start, when the tool still edits the same object. + fn batch_notice(&mut self, what: &str) { + match what { + "batch.begin" => self.batches.push(HistoryState { + editing: self.editing, + caret: self.caret, + anchor: self.anchor, + goal_x: self.goal_x, + known: self.known.clone(), + edits: self.edits.clone(), + stepped: self.stepped, + }), + "batch.end" => { + self.batches.pop(); + } + "batch.rollBack" => { + if let Some(s) = self.batches.pop() + && s.editing == self.editing + { + (self.caret, self.anchor, self.goal_x) = (s.caret, s.anchor, s.goal_x); + (self.known, self.edits, self.stepped) = (s.known, s.edits, s.stepped); + } + } + _ => {} + } + } + /// After Undo of a list that typed characters started (and of what was typed after it), the /// text is as it was typed: the caret goes on after the characters. After Redo the text is as /// the list left it: the caret goes back to the paragraph's start. diff --git a/crates/tools/src/text/tool.rs b/crates/tools/src/text/tool.rs index 455a44b65..f2bfff400 100644 --- a/crates/tools/src/text/tool.rs +++ b/crates/tools/src/text/tool.rs @@ -468,10 +468,13 @@ impl Tool for TypeTool { self.finish(cx) } /// After `text.create` / `text.createInPath` the engine selects the new object; edit it. After - /// `edit.undo` / `edit.redo` the caret goes to where the step changed the edited text. + /// `edit.undo` / `edit.redo` the caret goes to where the step changed the edited text. The + /// `batch.*` notices keep what a failed `command.batch` changed from staying ([`TypeTool::batch_notice`]). fn notify(&mut self, cx: &ToolContext, what: &str) { if matches!(what, "edit.undo" | "edit.redo") { self.after_history_step(cx, what == "edit.undo"); + } else if what.starts_with("batch.") { + self.batch_notice(what); } else if what == "text.editNew" && let Some(id) = cx.selection.objects.first().copied() {