diff --git a/crates/engine/src/cmd/inspect.rs b/crates/engine/src/cmd/inspect.rs index f92b48bd9..366d8e6e6 100644 --- a/crates/engine/src/cmd/inspect.rs +++ b/crates/engine/src/cmd/inspect.rs @@ -11,7 +11,7 @@ pub fn specs() -> Vec { cmd!(query "sheet.read", "Read Range", [], None, "{range?: \"A1:D10\" (default: used range), sheet?, formulas?: bool, formatted?: bool} → rows of values", has_doc, read), cmd!(query "cell.get", "Get Cell", [], None, "{cell?, sheet?} → value, formula, display text, style, comment, link", has_doc, cell_get), cmd!(query "selection.stats", "Selection Statistics", [], None, "{} → count, sum, average, min, max", has_doc, sel_stats), - cmd!(query "history.list", "Undo History", [], None, "{} → undo/redo labels", has_doc, history), + cmd!(query "history.list", "Undo History", [], None, "{} → undo/redo labels, grouping (an undo group is open)", has_doc, history), cmd!(query "app.commands", "List Commands", [], None, "{search?} → ids, labels, params", always, list_commands), ] } @@ -148,9 +148,11 @@ fn sel_stats(s: &mut Session, _: &Json) -> Result { fn history(s: &mut Session, _: &Json) -> Result { let d = s.doc()?; - Ok( - json!({"undo": d.undo.iter().rev().map(|e| e.label.clone()).collect::>(), "redo": d.redo.iter().rev().map(|e| e.label.clone()).collect::>()}), - ) + Ok(json!({ + "undo": d.undo.iter().rev().map(|e| e.label.clone()).collect::>(), + "redo": d.redo.iter().rev().map(|e| e.label.clone()).collect::>(), + "grouping": s.in_group(d.uid), + })) } fn list_commands(s: &mut Session, p: &Json) -> Result { diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 50865a2b7..d8abfac61 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -96,12 +96,19 @@ pub fn has_doc(s: &Session) -> std::result::Result<(), String> { } pub fn can_undo(s: &Session) -> std::result::Result<(), String> { has_doc(s)?; + not_grouping(s)?; if s.active().is_some_and(|d| !d.undo.is_empty()) { Ok(()) } else { Err("nothing to undo".into()) } } pub fn can_redo(s: &Session) -> std::result::Result<(), String> { has_doc(s)?; + not_grouping(s)?; if s.active().is_some_and(|d| !d.redo.is_empty()) { Ok(()) } else { Err("nothing to redo".into()) } } +/// Undo and Redo wait while an undo group is open on the active document (an action that spans +/// several calls is still running). +fn not_grouping(s: &Session) -> std::result::Result<(), String> { + if s.active().is_some_and(|d| s.in_group(d.uid)) { Err("finish or stop the running action first".into()) } else { Ok(()) } +} pub fn has_clipboard(s: &Session) -> std::result::Result<(), String> { has_doc(s)?; if s.clipboard.is_some() { Ok(()) } else { Err("the clipboard is empty".into()) } diff --git a/crates/engine/src/cmd/sheet.rs b/crates/engine/src/cmd/sheet.rs index 7870dee4e..c42b3abd4 100644 --- a/crates/engine/src/cmd/sheet.rs +++ b/crates/engine/src/cmd/sheet.rs @@ -554,11 +554,15 @@ fn insert_copied_cells(s: &mut Session, p: &Json) -> Result { } let revision = d.revision; let before = (d.wb.clone(), d.selection.clone()); + // Inside an undo group the nested steps add no undo entries of their own. + let nested_entries = !s.in_group(d.uid); let result = insert_copied_steps(s, &clip, sheet, at, shift, cut); let d = s.doc_mut()?; // The nested steps collapse into this command's single undo step. - for _ in 0..d.revision.saturating_sub(revision) { - d.undo.pop(); + if nested_entries { + for _ in 0..d.revision.saturating_sub(revision) { + d.undo.pop(); + } } match result { Ok((final_range, out)) => { diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 258a1cb72..e8e3d73d6 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -63,6 +63,36 @@ pub struct HistoryEntry { pub selection: Selection, } +/// An undo group opened with [`Session::group_begin`]. Hand it back to [`Session::group_end`] +/// (keep the changes as one undo step) or [`Session::group_rollback`] (undo them all). +#[derive(Debug, PartialEq, Eq)] +#[must_use = "an undo group stays open until it is ended or rolled back"] +pub struct GroupToken { + id: u64, + doc: u64, +} + +impl GroupToken { + /// The uid of the document the group belongs to. + pub fn doc(&self) -> u64 { + self.doc + } +} + +/// An open undo group: what runs on `doc` until it ends becomes one undo step. +#[derive(Clone, Debug)] +struct OpenGroup { + id: u64, + doc: u64, + label: String, + /// Undo depth when the group began; the group's own steps are cut back to it. + start: usize, + wb: Arc, + selection: Selection, + /// The redo stack when the group began (a rollback puts it back). + redo: Vec, +} + /// The clipboard: a block of cells (with formulas and styles) or marching-ants reference. #[derive(Clone, Debug)] pub struct Clipboard { @@ -256,6 +286,11 @@ pub struct Session { /// Review › Notes › Show All Notes: every note is displayed. An application display setting /// (like Excel's comment indicator option), so it's not saved in the file. pub show_all_notes: bool, + /// Open undo groups, innermost last. + groups: Vec, + next_group: u64, + /// Counts group begins and ends, so `execute` can tell that a command opened or closed one. + group_epoch: u64, } impl Session { @@ -292,7 +327,8 @@ impl Session { } pub fn close_document(&mut self, i: usize) { if i < self.docs.len() { - self.docs.remove(i); + let d = self.docs.remove(i); + self.groups.retain(|g| g.doc != d.uid); if self.active >= self.docs.len() { self.active = self.docs.len().saturating_sub(1); } @@ -313,7 +349,8 @@ impl Session { command_specs().iter().map(|c| c.info(self)).collect() } - /// Runs a command by id. Undoable commands record a history step when the workbook changed. + /// Runs a command by id. Undoable commands record a history step when the workbook changed, + /// unless an undo group is open on the document (the group then makes the step). /// Panics inside commands are caught and reported; the workbook is restored. pub fn execute(&mut self, id: &str, params: Json) -> Result { let spec = find_command(id).ok_or_else(|| EngineError::UnknownCommand(id.to_string()))?; @@ -322,13 +359,15 @@ impl Session { } let before = self.active().map(|d| (d.uid, d.wb.clone(), d.selection.clone())); let params = if params.is_null() { Json::Object(Default::default()) } else { params }; + let (first_group, group_epoch) = (self.next_group, self.group_epoch); let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| (spec.run)(self, ¶ms))); let result = match result { Ok(r) => r, Err(p) => { let msg = p.downcast_ref::<&str>().map(|s| s.to_string()).or_else(|| p.downcast_ref::().cloned()).unwrap_or_else(|| "panic".into()); - // Restore the workbook as it was. + // Restore the workbook as it was, and drop the undo groups the command opened. + self.groups.retain(|g| g.id < first_group); if let Some((uid, wb, sel)) = &before && let Some(d) = self.docs.iter_mut().find(|d| d.uid == *uid) { @@ -345,14 +384,18 @@ impl Session { self.journal.drain(..5_000); } } + // Inside an undo group, or when the command opened or closed one, the group owns the step. + let grouped = before.as_ref().is_some_and(|(uid, ..)| self.group_epoch != group_epoch || self.in_group(*uid)); if spec.undoable && let Some((uid, wb, sel)) = before && let Some(d) = self.docs.iter_mut().find(|d| d.uid == uid) && !Arc::ptr_eq(&d.wb, &wb) { - d.undo.push(HistoryEntry { label: spec.label.trim_end_matches('…').to_string(), wb, selection: sel }); - if d.undo.len() > UNDO_LIMIT { - d.undo.remove(0); + if !grouped { + d.undo.push(HistoryEntry { label: spec.label.trim_end_matches('…').to_string(), wb, selection: sel }); + if d.undo.len() > UNDO_LIMIT { + d.undo.remove(0); + } } d.redo.clear(); d.revision += 1; @@ -360,6 +403,100 @@ impl Session { result } + /// Runs `f` as one undo step labelled `label` on the active document: whatever commands `f` + /// runs, one Undo brings back the workbook as it was before. If `f` fails, every change it made + /// is rolled back and its error is returned. Groups nest; the outermost one makes the step. + pub fn grouped(&mut self, label: &str, f: impl FnOnce(&mut Session) -> Result) -> Result { + let token = self.group_begin(label)?; + match f(self) { + Ok(r) => { + self.group_end(token)?; + Ok(r) + } + Err(e) => { + // The error from `f` is the one to report. A rollback fails only when the group + // has already ended, and then there is nothing left to undo. + let _ = self.group_rollback(token); + Err(e) + } + } + } + + /// Opens an undo group on the active document that can span several calls (for example a + /// debugger that runs one step per call). Until [`Session::group_end`] or + /// [`Session::group_rollback`], undoable commands on that document add no undo steps of their + /// own, and Undo and Redo are unavailable there. + pub fn group_begin(&mut self, label: &str) -> Result { + let d = self.doc()?; + let g = OpenGroup { + id: self.next_group, + doc: d.uid, + label: label.trim_end_matches('…').to_string(), + start: d.undo.len(), + wb: d.wb.clone(), + selection: d.selection.clone(), + redo: d.redo.clone(), + }; + let token = GroupToken { id: g.id, doc: g.doc }; + self.next_group = self.next_group.saturating_add(1); + self.group_epoch = self.group_epoch.wrapping_add(1); + self.groups.push(g); + Ok(token) + } + + /// Ends an undo group. If the workbook changed, the group becomes one undo step, unless it is + /// nested in another open group on the same document, which then owns the step. Groups opened + /// inside it and still open end with it. Returns whether the workbook changed. + pub fn group_end(&mut self, token: GroupToken) -> Result { + let g = self.take_group(&token)?; + let outer = self.in_group(g.doc); + let Some(d) = self.docs.iter_mut().find(|d| d.uid == g.doc) else { return Ok(false) }; + // Steps pushed directly while the group was open fold into the group's step. + d.undo.truncate(g.start); + let changed = !Arc::ptr_eq(&d.wb, &g.wb); + if changed && !outer { + d.undo.push(HistoryEntry { label: g.label, wb: g.wb, selection: g.selection }); + while d.undo.len() > UNDO_LIMIT { + d.undo.remove(0); + } + d.redo.clear(); + d.revision += 1; + } + Ok(changed) + } + + /// Ends an undo group by putting back the workbook, selection, undo and redo history as they + /// were when it began. + pub fn group_rollback(&mut self, token: GroupToken) -> Result<()> { + let g = self.take_group(&token)?; + let Some(d) = self.docs.iter_mut().find(|d| d.uid == g.doc) else { return Ok(()) }; + if !Arc::ptr_eq(&d.wb, &g.wb) { + d.wb = g.wb; + d.calc.rebuild(&d.wb); + } + d.selection = g.selection; + d.undo.truncate(g.start); + d.redo = g.redo; + d.revision += 1; + Ok(()) + } + + /// Whether an undo group is open on the document with this uid. + pub fn in_group(&self, doc: u64) -> bool { + self.groups.iter().any(|g| g.doc == doc) + } + + /// Removes the token's group and any group on the same document opened after it. + fn take_group(&mut self, token: &GroupToken) -> Result { + let ended = || EngineError::Other("That undo group has already ended.".into()); + let at = self.groups.iter().position(|g| g.id == token.id).ok_or_else(ended)?; + let mut later = self.groups.split_off(at).into_iter(); + let g = later.next().ok_or_else(ended)?; + self.groups.extend(later.filter(|o| o.doc != g.doc)); + self.group_epoch = self.group_epoch.wrapping_add(1); + Ok(g) + } + pub fn run(&mut self, id: &str, params: Json) -> std::result::Result { self.execute(id, params).map_err(|e| e.to_string()) } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 2ac290654..b5ac30d43 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -3344,3 +3344,4 @@ fn csv_as_text_keeps_identifier_columns_as_written() { let saved = s.execute("file.saveBytes", json!({"format": "csv"})).unwrap(); assert_eq!(crate::io::base64_decode(saved["base64"].as_str().unwrap()).unwrap(), csv.replace('\n', "\r\n").as_bytes()); } +mod seams; diff --git a/crates/engine/src/tests/seams.rs b/crates/engine/src/tests/seams.rs new file mode 100644 index 000000000..aec2c8544 --- /dev/null +++ b/crates/engine/src/tests/seams.rs @@ -0,0 +1,203 @@ +//! Engine building blocks for multi-command actions such as macros: undo groups, the recording +//! buffer, document trust and chunked storage of workbook data in hidden names. + +use serde_json::json; + +use super::{s, v}; +use crate::{EngineError, Session}; + +fn undo_labels(s: &Session) -> Vec { + s.doc().unwrap().undo.iter().map(|e| e.label.clone()).collect() +} + +fn set(s: &mut Session, cell: &str, input: &str) { + s.execute("cell.set", json!({"cell": cell, "input": input})).unwrap(); +} + +// ---------------------------------------------------------------- undo groups + +#[test] +fn grouped_commands_are_one_undo_step() { + let mut s = s(); + set(&mut s, "A1", "before"); + let r = s.grouped("Fill Three", |s| { + set(s, "A1", "1"); + set(s, "A2", "2"); + s.execute("home.bold", json!({"range": "A1:A2"}))?; + Ok(7) + }); + assert_eq!(r.unwrap(), 7); + assert_eq!(undo_labels(&s), ["Enter Cell", "Fill Three"]); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(v(&s, "A1").display(), "before"); + assert!(v(&s, "A2").is_empty()); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(v(&s, "A2").display(), "2"); + assert!(!s.in_group(s.doc().unwrap().uid)); +} + +#[test] +fn a_failing_group_rolls_everything_back() { + let mut s = s(); + set(&mut s, "B1", "base"); + set(&mut s, "A1", "undone"); + s.execute("edit.undo", json!({})).unwrap(); + s.execute("selection.set", json!({"range": "C3"})).unwrap(); + let redo_before = s.doc().unwrap().redo.len(); + assert_eq!(redo_before, 1); + let r: crate::Result<()> = s.grouped("Doomed", |s| { + set(s, "B1", "changed"); + s.execute("selection.set", json!({"range": "D4"}))?; + set(s, "B2", "new"); + Err(EngineError::Other("step 3 failed".into())) + }); + assert!(r.unwrap_err().to_string().contains("step 3 failed")); + assert_eq!(v(&s, "B1").display(), "base"); + assert!(v(&s, "B2").is_empty()); + assert_eq!(s.doc().unwrap().selection.a1(), "C3"); + assert_eq!(undo_labels(&s), ["Enter Cell"]); + assert_eq!(s.doc().unwrap().redo.len(), redo_before, "the redo stack comes back too"); + // The recalculation follows the restored workbook. + set(&mut s, "C1", "=B1&\"!\""); + assert_eq!(v(&s, "C1").display(), "base!"); +} + +#[test] +fn groups_nest_and_the_outermost_makes_the_step() { + let mut s = s(); + let r = s.grouped("Outer", |s| { + set(s, "A1", "1"); + s.grouped("Inner", |s| { + set(s, "A2", "2"); + Ok(()) + })?; + // A failing inner group rolls back only its own changes; the outer one carries on. + let inner: crate::Result<()> = s.grouped("Inner fails", |s| { + set(s, "A3", "3"); + Err(EngineError::Other("no".into())) + }); + assert!(inner.is_err()); + assert!(v(s, "A3").is_empty()); + set(s, "A4", "4"); + Ok(()) + }); + r.unwrap(); + assert_eq!(undo_labels(&s), ["Outer"]); + assert_eq!(v(&s, "A2").display(), "2"); + s.execute("edit.undo", json!({})).unwrap(); + for c in ["A1", "A2", "A3", "A4"] { + assert!(v(&s, c).is_empty(), "{c}"); + } +} + +#[test] +fn an_unchanged_group_adds_no_step() { + let mut s = s(); + set(&mut s, "A1", "x"); + s.grouped("Look Around", |s| { + s.execute("selection.set", json!({"range": "B2"}))?; + s.execute("cell.get", json!({"cell": "A1"}))?; + Ok(()) + }) + .unwrap(); + assert_eq!(undo_labels(&s), ["Enter Cell"]); +} + +#[test] +fn a_group_spans_calls_and_blocks_undo_until_it_ends() { + let mut s = s(); + set(&mut s, "A1", "start"); + let token = s.group_begin("Debug Macro").unwrap(); + // Each step is its own call, as a debugger makes them. + set(&mut s, "A1", "step 1"); + set(&mut s, "A2", "step 2"); + let undo = s.execute("edit.undo", json!({})); + assert!(matches!(undo, Err(EngineError::Disabled(..))), "{undo:?}"); + assert!(matches!(s.execute("edit.redo", json!({})), Err(EngineError::Disabled(..)))); + assert_eq!(s.execute("history.list", json!({})).unwrap()["grouping"], json!(true)); + assert!(s.group_end(token).unwrap()); + assert_eq!(s.execute("history.list", json!({})).unwrap()["grouping"], json!(false)); + assert_eq!(undo_labels(&s), ["Enter Cell", "Debug Macro"]); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(v(&s, "A1").display(), "start"); + assert!(v(&s, "A2").is_empty()); +} + +#[test] +fn rolling_back_a_spanning_group_restores_the_start() { + let mut s = s(); + set(&mut s, "A1", "start"); + let token = s.group_begin("Debug Macro").unwrap(); + set(&mut s, "A1", "step 1"); + s.execute("home.insertSheet", json!({})).unwrap(); + s.group_rollback(token).unwrap(); + assert_eq!(v(&s, "A1").display(), "start"); + assert_eq!(s.doc().unwrap().wb.sheets.len(), 1); + assert_eq!(undo_labels(&s), ["Enter Cell"]); +} + +#[test] +fn ending_an_outer_group_ends_the_inner_ones() { + let mut s = s(); + let outer = s.group_begin("Outer").unwrap(); + let inner = s.group_begin("Inner").unwrap(); + set(&mut s, "A1", "1"); + s.group_end(outer).unwrap(); + assert_eq!(undo_labels(&s), ["Outer"]); + assert!(s.group_end(inner).is_err(), "the inner group ended with its outer group"); + assert!(!s.in_group(s.doc().unwrap().uid)); +} + +#[test] +fn a_long_group_keeps_the_history_before_it() { + let mut s = s(); + for i in 0..100 { + set(&mut s, "A1", &i.to_string()); + } + let first = s.doc().unwrap().undo.first().map(|e| e.wb.clone()).unwrap(); + s.grouped("Many Steps", |s| { + for i in 0..250 { + set(s, "B1", &i.to_string()); + } + Ok(()) + }) + .unwrap(); + let d = s.doc().unwrap(); + assert_eq!(d.undo.len(), 100, "the undo limit still holds"); + assert_eq!(d.undo.last().map(|e| e.label.as_str()), Some("Many Steps")); + // Only the oldest entry made room for the group's one step. + assert!(!d.undo.iter().any(|e| std::sync::Arc::ptr_eq(&e.wb, &first))); + assert_eq!(d.undo.iter().filter(|e| e.label == "Enter Cell").count(), 99); +} + +#[test] +fn closing_a_document_drops_its_groups() { + let mut s = s(); + let token = s.group_begin("Debug Macro").unwrap(); + let uid = token.doc(); + s.close_document(s.active_index()); + assert!(!s.in_group(uid)); + assert!(s.group_end(token).is_err()); + assert!(matches!(s.group_begin("x"), Err(EngineError::NoDocument))); +} + +#[test] +fn a_group_owns_the_steps_of_commands_that_run_commands() { + // Insert Copied Cells runs nested steps and folds them into its own undo step; inside a group + // it must leave the history from before the group alone. + let mut s = s(); + set(&mut s, "A1", "kept"); + set(&mut s, "A5", "copied"); + s.execute("edit.copy", json!({"range": "5:5"})).unwrap(); + s.grouped("Insert Inside", |s| { + s.execute("selection.set", json!({"range": "A2"}))?; + s.execute("home.insertCopiedCells", json!({}))?; + Ok(()) + }) + .unwrap(); + assert_eq!(v(&s, "A2").display(), "copied"); + assert_eq!(undo_labels(&s), ["Enter Cell", "Enter Cell", "Insert Inside"]); + s.execute("edit.undo", json!({})).unwrap(); + assert!(v(&s, "A2").is_empty()); + assert_eq!(v(&s, "A1").display(), "kept"); +}