diff --git a/crates/snowbound/src/undo.rs b/crates/snowbound/src/undo.rs index 39172b1b05ad348bb27d7ef4f7685f8759d8ccc4..2c5b5e82b7e185e049a38c733304ff660980b370 100644 --- a/crates/snowbound/src/undo.rs +++ b/crates/snowbound/src/undo.rs @@ -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, + taken: Option<(bool, usize)>, ) -> Result<(), Box> { 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( - session - .library - .structure - .try_lock_arc() - .ok_or_else(|| std::io::Error::from(std::io::ErrorKind::WouldBlock))?, - ) + let Some(changing) = session.library.structure.try_lock_arc() else { + if let Some(taken) = taken { + self.undo.put_back( + taken, + Action { + 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) -> bool { - matches!(next, Some(Next::Action(_))) + matches!(next, Some(Next::Action(..))) } fn is_edit(next: Option) -> 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 { - change: Change::Rename { .. }, - .. - })) + Some(Next::Action( + 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 {