Commit 96172e13a7a316a5fc038b61aa836d3f91d8a837 | snowbound

author	clover caruso <git@paperclover.net> 2026-10-10 21:32:55-07:00
committer	clover caruso <git@paperclover.net> 2026-10-10 22:56:06-07:00
log	96172e13a7a316a5fc038b61aa836d3f91d8a837
tree	4c9b8891cb0a35749d0ecc26a603796398983fc6
parent	b28c7cbe0b6bd7420e717e2005265ecbea3a37de
signature	Signed by SSH key SHA256:52mNGHRsVFBDED9IAX5pe+LRWUefqTbxEReunq21QvU (~clover)

fix: keep Undo and Redo history intact when recycling is busy

Carry the removed action position through preflight and restore the original action to its original stack when structural ownership is unavailable. A retry must remain behind newer edits on other pages. Restoration happens only before work starts; uncertain asynchronous publication is not replayed.

Cover contention and exact history ordering for both Undo and Redo.

Fixes #119
Assisted-by: gpt-6.1-sol

1 files changed, 86 insertions(+), 23 deletions(-)

crates
- snowbound
  - src
    undo.rs

crates/snowbound/src/undo.rs+86-23

 @@ -145,7 +145,8 @@ impl Action {
 pub enum Next {
     /// The open page's last edit, or the last undone.
     Edit,
-    Action(Action),
+    /// An action and its original position until preflight accepts it.
+    Action(Action, usize),
     /// The page a created page Undo would take out still holds edits made since, so
     /// Undo shows it, where those go back first.
     Show(ExGuid),
 @@ -168,6 +169,12 @@ impl Timeline {
         }
     }
+    /// Restores a refused action before newer edits on other pages, in its original stack.
+    fn put_back(&mut self, (redo, at): (bool, usize), action: Action) {
+        let steps = if redo { &mut self.redo } else { &mut self.undo };
+        steps.insert(at, Step::Action(action));
+    }
     /// Follows page `page`'s history, `depth` steps of Undo and Redo, after an edit or
     /// before one is taken: steps it gained are the newest; `run`, a run of typing that
     /// went on, makes its last step the newest.
 @@ -232,7 +239,7 @@ impl Timeline {
             Step::Action(_) => {}
         }
         match steps.remove(at) {
-            Step::Action(action) => Some(Next::Action(action)),
+            Step::Action(action) => Some(Next::Action(action, at)),
             Step::Edit(_) => unreachable!("an edit is not taken off"),
         }
     }
 @@ -988,7 +995,7 @@ impl State {
                 }
             }
             Some(Next::Show(page)) => self.commands.push(Command::OpenPage(page)),
-            Some(Next::Action(action)) => self.perform(action, Some(redo))?,
+            Some(Next::Action(action, at)) => self.perform(action, Some((redo, at)))?,
         }
         Ok(())
     }
 @@ -998,7 +1005,7 @@ impl State {
     pub(crate) fn perform(
         &mut self,
         action: Action,
-        taken: Option<bool>,
+        taken: Option<(bool, usize)>,
     ) -> Result<(), Box<dyn Error>> {
         let Action {
             notebook,
 @@ -1014,7 +1021,7 @@ impl State {
                 .ok_or("That notebook is closed")?;
             if !holds(&library.themes(), &from) {
                 return match taken {
-                    Some(redo) => self.take(redo),
+                    Some((redo, _)) => self.take(redo),
                     None => Ok(()),
                 };
             }
 @@ -1038,7 +1045,7 @@ impl State {
                 section,
                 change: Change::Themes { from: to, to: from },
             };
-            self.undo.note(taken, undo);
+            self.undo.note(taken.map(|(redo, _)| redo), undo);
             return self.wear_theme(true);
         }
         let Some(section) = section else {
 @@ -1050,11 +1057,11 @@ impl State {
                 .ok_or("That notebook is closed")?;
             let Some((structure, undo)) = restructuring(&library, &change) else {
                 return match taken {
-                    Some(redo) => self.take(redo),
+                    Some((redo, _)) => self.take(redo),
                     None => Ok(()),
                 };
             };
-            self.undo.note(taken, undo);
+            self.undo.note(taken.map(|(redo, _)| redo), undo);
             self.restructure(library, structure);
             return Ok(());
         };
 @@ -1065,13 +1072,20 @@ impl State {
             Change::Delete { created: false, .. } | Change::Restore { created: false, .. }
         ) && matches!(session.library.notebook, Ok(Some(_)))
         {
-            Some(
+            let Some(changing) = session.library.structure.try_lock_arc() else {
-                session
+                if let Some(taken) = taken {
-                    .library
+                    self.undo.put_back(
-                    .structure
+                        taken,
-                    .try_lock_arc()
+                        Action {
-                    .ok_or_else(|| std::io::Error::from(std::io::ErrorKind::WouldBlock))?,
+                            notebook,
-            )
+                            section: Some(section),
+                            change,
+                        },
+                    );
+                }
+                return Err(std::io::Error::from(std::io::ErrorKind::WouldBlock).into());
+            };
+            Some(changing)
         } else {
             None
         };
 @@ -1125,7 +1139,7 @@ impl State {
                 let Some(applied) = applied else {
                     // What it would change is gone: Undo goes on to the step before.
                     return match taken {
-                        Some(redo) if stale => {
+                        Some((redo, _)) if stale => {
                             state.undo.waiting.push_front(redo);
                             Ok(())
                         }
 @@ -1138,7 +1152,7 @@ impl State {
                     section: Some(section),
                     change: applied.undo,
                 };
-                state.undo.note(taken, undo);
+                state.undo.note(taken.map(|(redo, _)| redo), undo);
                 state.edited(Vec::new());
                 Ok(())
             })));
 @@ -1207,7 +1221,7 @@ mod tests {
     }
     fn is_action(next: Option<Next>) -> bool {
-        matches!(next, Some(Next::Action(_)))
+        matches!(next, Some(Next::Action(..)))
     }
     fn is_edit(next: Option<Next>) -> bool {
 @@ -1288,10 +1302,13 @@ mod tests {
         assert!(!timeline.reaches(false, &other));
         assert!(matches!(
             timeline.next(false, page(7), [0, 0], &elsewhere),
-            Some(Next::Action(Action {
+            Some(Next::Action(
-                change: Change::Rename { .. },
+                Action {
-                ..
+                    change: Change::Rename { .. },
-            }))
+                    ..
+                },
+                _
+            ))
         ));
         assert!(timeline.next(false, page(7), [0, 0], &elsewhere).is_none());
         assert!(is_action(timeline.next(false, page(7), [0, 0], &here())));
 @@ -1306,7 +1323,7 @@ mod tests {
         let mut timeline = Timeline::default();
         let shown = page(1);
         timeline.record(created(page(2)));
-        let Some(Next::Action(action)) = timeline.next(false, shown, [0, 0], &here()) else {
+        let Some(Next::Action(action, _)) = timeline.next(false, shown, [0, 0], &here()) else {
             panic!()
         };
         timeline.note(Some(false), action);
 @@ -1320,6 +1337,52 @@ mod tests {
         assert!(!timeline.reaches(true, &here()));
     }
+    #[test]
+    fn contended_page_actions_keep_their_position_in_undo_and_redo() {
+        for redo in [false, true] {
+            let mut timeline = Timeline::default();
+            let action = Action {
+                change: Change::Delete {
+                    pages: vec![page(2)],
+                    show: None,
+                    created: false,
+                },
+                ..created(page(2))
+            };
+            let steps = if redo {
+                &mut timeline.redo
+            } else {
+                &mut timeline.undo
+            };
+            steps.push(Step::Action(action));
+            steps.push(Step::Edit(page(3)));
+            let Some(Next::Action(action, at)) = timeline.next(redo, page(1), [0, 0], &here())
+            else {
+                panic!("the current page has a pending action");
+            };
+            let structure = Arc::new(async_lock::Mutex::new(()));
+            let changing = structure.try_lock_arc().unwrap();
+            assert!(structure.try_lock_arc().is_none());
+            timeline.put_back((redo, at), action);
+            let depth = if redo { [0, 1] } else { [1, 0] };
+            assert!(
+                is_edit(timeline.next(redo, page(3), depth, &here())),
+                "newer edit stays first"
+            );
+            timeline.stepped(page(3), redo);
+            drop(changing);
+            let _changing = structure.try_lock_arc().unwrap();
+            let Some(Next::Action(action, at)) = timeline.next(redo, page(1), [0, 0], &here())
+            else {
+                panic!("the refused action must remain in its original stack");
+            };
+            assert_eq!(at, 0);
+            assert!(
+                matches!(action.change, Change::Delete { pages, created: false, .. } if pages == [page(2)])
+            );
+        }
+    }
     const AUTHOR: &str = "Rust Author";
     fn dated() -> PageCreation {