Merge pull request #1133 from g1obalworlds/fix-batch-rollback-closed-tabs

engine: restore tabs closed inside a failed command group

crates/engine/src/batch_rollback_tests.rs

@@ -34,6 +34,31 @@ fn failed_group_closes_documents_opened_inside_it() {
     assert_eq!(s.journal, before_journal);
 }
 
+#[test]
+fn failed_group_reopens_documents_closed_inside_it() {
+    let mut s = Session::new();
+    s.execute("file.new", &json!({"name": "Second"})).unwrap();
+    let uids: Vec<_> = s.docs.iter().map(|ds| ds.uid).collect();
+    assert_eq!((s.docs.len(), s.active), (2, 1));
+
+    let result = s.execute(
+        "command.batch",
+        &json!({
+            "commands": [
+                {"command": "file.close", "params": {"index": 1, "force": true}},
+                {"command": "node.set", "params": {"id": 999999, "props": {"name": "must fail"}}}
+            ]
+        }),
+    );
+
+    assert!(result.is_err());
+    assert_eq!(s.docs.len(), 2);
+    assert_eq!(s.active, 1);
+    assert_eq!(s.docs.iter().map(|ds| ds.uid).collect::<Vec<_>>(), uids);
+    assert!(s.ds().is_ok());
+    assert!(s.closed_in_group.is_empty());
+}
+
 #[test]
 fn failed_plugin_run_restores_each_document_without_restoring_unrelated_state() {
     let mut s = Session::new();

crates/engine/src/branches.rs

@@ -194,7 +194,7 @@ pub fn tab_of(s: &Session, main: u64, id: &str) -> Option<usize> {
 /// branch's). Their edits are already in the branches ([`sync`]).
 pub(crate) fn close_tabs(s: &mut Session, main: u64, id: Option<&str>) {
     let active = s.docs.get(s.active).map(|d| d.uid);
-    s.docs.retain(|d| !d.branch_of.as_ref().is_some_and(|(m, b)| *m == main && id.is_none_or(|i| i == b)));
+    s.remove_tabs(|d| d.branch_of.as_ref().is_some_and(|(m, b)| *m == main && id.is_none_or(|i| i == b)));
     s.active = active.and_then(|a| s.docs.iter().position(|d| d.uid == a)).unwrap_or(s.active).min(s.docs.len().saturating_sub(1));
 }
 

crates/engine/src/cmd/file.rs

@@ -1756,7 +1756,7 @@ fn close(s: &mut Session, p: &Value) -> Result<Value> {
     }
     let uid = ds.uid;
     let active_uid = s.docs.get(s.active).map(|d| d.uid);
-    s.docs.remove(i);
+    s.remove_tabs(|d| d.uid == uid);
     // Removing a tab before the active one shifts its index; keep the same document selected.
     s.active = active_uid.and_then(|a| s.docs.iter().position(|d| d.uid == a)).unwrap_or(i.min(s.docs.len().saturating_sub(1)));
     crate::branches::close_tabs(s, uid, None);

crates/engine/src/lib.rs

@@ -812,6 +812,10 @@ pub struct Session {
     /// Command log (id + params) for actions/automation. File imports that carry the file bytes
     /// (`file.import` with `data`) are not logged, so the log never holds a copy of a pasted file.
     pub journal: Vec<(String, Value)>,
+    /// Nesting depth of [`Session::grouped`]; tabs closed while it is above zero are kept in
+    /// `closed_in_group` so a failed group can bring them back.
+    group_depth: usize,
+    closed_in_group: Vec<DocState>,
     /// Recently used colours.
     pub recent_colors: Vec<figcraft_color::Color>,
     /// The last undoable command executed through [`Session::execute`], excluding "Repeat".
@@ -867,6 +871,8 @@ impl Session {
             ui_requests: Vec::new(),
             ui_dark: None,
             journal: Vec::new(),
+            group_depth: 0,
+            closed_in_group: Vec::new(),
             recent_colors: Vec::new(),
             last_command: None,
             message: None,
@@ -1306,6 +1312,16 @@ impl Session {
         Ok(())
     }
 
+    /// Take the tabs `pred` matches out of `docs`; inside a group they are kept so that a failed
+    /// group can reopen them.
+    pub(crate) fn remove_tabs(&mut self, mut pred: impl FnMut(&DocState) -> bool) {
+        let (gone, keep): (Vec<_>, Vec<_>) = std::mem::take(&mut self.docs).into_iter().partition(|d| pred(d));
+        self.docs = keep;
+        if self.group_depth > 0 {
+            self.closed_in_group.extend(gone);
+        }
+    }
+
     /// Run `f` as one undo step called `label`: the commands it executes push no steps of their
     /// own, and when it fails the documents stay at their group-start state.
     pub fn grouped<T>(&mut self, label: &str, f: impl FnOnce(&mut Self) -> Result<T>) -> Result<T> {
@@ -1324,13 +1340,23 @@ impl Session {
         for (uid, _, _, _, _, _) in &before {
             self.live_hold(*uid, true);
         }
+        self.group_depth += 1;
         let result = f(self);
+        self.group_depth -= 1;
         for (uid, _, _, _, _, _) in &before {
             self.live_hold(*uid, false);
         }
 
         if result.is_err() {
-            self.docs.retain(|ds| before.iter().any(|(uid, _, _, _, _, _)| *uid == ds.uid));
+            // Tabs the group closed come back from the stash, and the group-start order returns.
+            let mut current = std::mem::take(&mut self.docs);
+            for (uid, ..) in &before {
+                let tab = match current.iter().position(|d| d.uid == *uid) {
+                    Some(i) => Some(current.swap_remove(i)),
+                    None => self.closed_in_group.iter().position(|d| d.uid == *uid).map(|i| self.closed_in_group.swap_remove(i)),
+                };
+                self.docs.extend(tab);
+            }
             for ((uid, doc, selection, page, undo_len, limit), redo) in before.iter().zip(redo_before) {
                 if let Some(ds) = self.docs.iter_mut().find(|ds| ds.uid == *uid) {
                     ds.history.redo = redo;
@@ -1343,8 +1369,14 @@ impl Session {
             }
             self.active = active;
             self.journal.truncate(journal_len);
+            if self.group_depth == 0 {
+                self.closed_in_group.clear();
+            }
             return result;
         }
+        if self.group_depth == 0 {
+            self.closed_in_group.clear();
+        }
 
         let changed: Vec<_> = before
             .iter()