script.run: roll a failed script back instead of stranding its edits

A script whose later step failed kept the successful prefix in the document
but truncated the prefix's undo entries (and returned before the command's
aggregate undo entry was recorded), so the edits could not be undone at all.
A failed script now restores the document, selection and history from before
it started: all-or-nothing.

Regression test: frame.create followed by an unknown command is an error, the
document has no frame, nothing is left to undo, and the next edit undoes
normally. The one-undo-step success path still holds.
This commit is contained in:
midasmatterhorn committed 2026-10-14 23:57:21 +03:00
1 parent d07cbbe94c
commit a393ab2018
1 file changed
+34 -6
+34 -6
View File
@@ -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<Value> {
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);
}
}