diff --git a/crates/engine/src/cmd/scripts.rs b/crates/engine/src/cmd/scripts.rs index 0fa89c59..5b6d288a 100644 --- a/crates/engine/src/cmd/scripts.rs +++ b/crates/engine/src/cmd/scripts.rs @@ -1,5 +1,7 @@ //! Window › Utilities › Scripts: run a command script (see [`crate::script`]) as one undo step. +use std::sync::Arc; + use serde_json::{Value, json}; use super::{CommandSpec, always, bad, cmd, str_param}; @@ -8,15 +10,23 @@ use crate::Result; fn run(s: &mut crate::Session, p: &Value) -> Result { let text = str_param(p, "text").ok_or_else(|| bad("script.run", "`text` required"))?; let steps = crate::script::parse(text).map_err(|e| bad("script.run", e))?; - // The steps' own undo entries collapse into this command's single one. - let uid = s.active().map(|d| d.uid); - let before = s.active().map_or(0, |d| d.history.undo.len()); + // The steps' own undo entries collapse into this command's single one. A step that fails + // rolls the successful prefix back (document, selection and history), so a half-run script + // never strands edits that can't be undone (#709). + let before = s.active().map(|d| (d.uid, d.doc.clone(), d.selection.clone(), d.history.clone())); let report = s.run_script(&steps); if let Some(st) = s.active_mut() - && Some(st.uid) == uid - && st.history.undo.len() > before + && let Some((uid, doc, selection, history)) = &before + && st.uid == *uid { - st.history.undo.truncate(before); + if report.failed.is_some() { + st.doc = Arc::clone(doc); + st.selection = selection.clone(); + st.history = history.clone(); + st.revision = st.revision.saturating_add(1); + } else if st.history.undo.len() > history.undo.len() { + st.history.undo.truncate(history.undo.len()); + } } let out = json!({"results": report.results, "steps": steps.len()}); match report.failed { @@ -56,4 +66,22 @@ mod tests { assert_eq!(count(&s), 0, "one undo removes the whole script's work"); assert!(s.execute("script.run", &json!({"text": "nope.command"})).is_err()); } + + /// A script that fails after a successful step leaves the document and its history exactly as + /// they were: the half-run prefix is not stranded without an undo entry (#709). + #[test] + fn a_failed_script_rolls_the_successful_prefix_back() { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + let count = |s: &Session| s.doc().unwrap().doc.spreads[0].items.len(); + let err = s.execute("script.run", &json!({"text": "frame.create {\"rect\": [72, 72, 200, 200]}\nnope.command"})).unwrap_err(); + assert!(err.to_string().contains("step 1") && err.to_string().contains("nope.command"), "{err}"); + assert_eq!(count(&s), 0, "the successful prefix is rolled back"); + assert!(s.execute("edit.undo", &json!({})).is_err(), "nothing was left to undo"); + // The session keeps working and its undo history is as it was. + s.execute("frame.create", &json!({"rect": [72, 72, 200, 200]})).unwrap(); + assert_eq!(count(&s), 1); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(count(&s), 0); + } }