diff --git a/crates/engine/src/batch_rollback_tests.rs b/crates/engine/src/batch_rollback_tests.rs index 4b1ac2b1..298ca097 100644 --- a/crates/engine/src/batch_rollback_tests.rs +++ b/crates/engine/src/batch_rollback_tests.rs @@ -38,6 +38,7 @@ fn failed_group_closes_documents_opened_inside_it() { 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( @@ -53,7 +54,9 @@ fn failed_group_reopens_documents_closed_inside_it() { 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::>(), uids); assert!(s.ds().is_ok()); + assert!(s.closed_in_group.is_empty()); } #[test] diff --git a/crates/engine/src/branches.rs b/crates/engine/src/branches.rs index 8bff0568..a705e0b9 100644 --- a/crates/engine/src/branches.rs +++ b/crates/engine/src/branches.rs @@ -186,7 +186,7 @@ pub fn tab_of(s: &Session, main: u64, id: &str) -> Option { /// 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)); } diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 7380711f..6b8e3195 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -1396,7 +1396,7 @@ fn close(s: &mut Session, p: &Value) -> Result { } 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); diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index aaa022f1..f90928d0 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -784,6 +784,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, /// Recently used colours. pub recent_colors: Vec, /// The last undoable command executed through [`Session::execute`], excluding "Repeat". @@ -839,6 +843,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, @@ -1269,6 +1275,16 @@ impl Session { self.execute(cmd, &p) } + /// 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(&mut self, label: &str, f: impl FnOnce(&mut Self) -> Result) -> Result { @@ -1277,8 +1293,6 @@ impl Session { self.docs.iter().map(|ds| (ds.uid, ds.doc.clone(), ds.selection.clone(), ds.page, ds.history.undo.len(), ds.history.limit)).collect(); // An edit inside clears the redo steps; a failed group has to bring them back. let redo_before: Vec<_> = self.docs.iter().map(|ds| ds.history.redo.clone()).collect(); - // Tabs the group closes (or reorders) come back from this. - let tabs_before = self.docs.clone(); let active = self.active; let journal_len = self.journal.len(); @@ -1289,20 +1303,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() { + // Tabs the group closed come back from the stash, and the group-start order returns. let mut current = std::mem::take(&mut self.docs); - self.docs = tabs_before - .into_iter() - .map(|t| match current.iter().position(|d| d.uid == t.uid) { - Some(i) => current.swap_remove(i), - None => t, - }) - .collect(); + 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; @@ -1315,8 +1332,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()