diff --git a/helix-core/src/history.rs b/helix-core/src/history.rs index 697f29b4ec98..825092423e00 100644 --- a/helix-core/src/history.rs +++ b/helix-core/src/history.rs @@ -122,17 +122,32 @@ impl History { /// Returns the changes since the given revision composed into a transaction. /// Returns None if there are no changes between the current and given revisions. pub fn changes_since(&self, revision: usize) -> Option { - if self.at_root() || self.current >= revision { - return None; - } + use std::cmp::Ordering::*; - // The bounds are checked in the if condition above: - // `revision` is known to be `< self.current`. - self.revisions[revision..self.current] - .iter() - .map(|revision| &revision.transaction) - .cloned() - .reduce(|acc, transaction| acc.compose(transaction)) + match revision.cmp(&self.current) { + Equal => None, + Less => { + let mut child = self.revisions[revision].last_child?.get(); + let mut transaction = self.revisions[child].transaction.clone(); + while child != self.current { + child = self.revisions[child].last_child?.get(); + transaction = transaction.compose(self.revisions[child].transaction.clone()); + } + Some(transaction) + } + Greater => { + let mut inversion = self.revisions[revision].inversion.clone(); + let mut parent = self.revisions[revision].parent; + while parent != self.current { + parent = self.revisions[parent].parent; + if parent == 0 { + return None; + } + inversion = inversion.compose(self.revisions[parent].inversion.clone()); + } + Some(inversion) + } + } } /// Undo the last edit. diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 8af5a7e3e400..4eb9742ff70e 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -3302,7 +3302,7 @@ fn undo(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); for _ in 0..count { - if !doc.undo(view) { + if !doc.undo(view.id) { cx.editor.set_status("Already at oldest change"); break; } @@ -3313,7 +3313,7 @@ fn redo(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); for _ in 0..count { - if !doc.redo(view) { + if !doc.redo(view.id) { cx.editor.set_status("Already at newest change"); break; } @@ -3325,7 +3325,7 @@ fn earlier(cx: &mut Context) { let (view, doc) = current!(cx.editor); for _ in 0..count { // rather than doing in batch we do this so get error halfway - if !doc.earlier(view, UndoKind::Steps(1)) { + if !doc.earlier(view.id, UndoKind::Steps(1)) { cx.editor.set_status("Already at oldest change"); break; } @@ -3337,7 +3337,7 @@ fn later(cx: &mut Context) { let (view, doc) = current!(cx.editor); for _ in 0..count { // rather than doing in batch we do this so get error halfway - if !doc.later(view, UndoKind::Steps(1)) { + if !doc.later(view.id, UndoKind::Steps(1)) { cx.editor.set_status("Already at newest change"); break; } diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index b8f99ff369d2..475f14d1de3c 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -481,7 +481,7 @@ fn earlier( let uk = args.join(" ").parse::().map_err(|s| anyhow!(s))?; let (view, doc) = current!(cx.editor); - let success = doc.earlier(view, uk); + let success = doc.earlier(view.id, uk); if !success { cx.editor.set_status("Already at oldest change"); } @@ -500,7 +500,7 @@ fn later( let uk = args.join(" ").parse::().map_err(|s| anyhow!(s))?; let (view, doc) = current!(cx.editor); - let success = doc.later(view, uk); + let success = doc.later(view.id, uk); if !success { cx.editor.set_status("Already at newest change"); } diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index 44f89b77ed15..737125031e99 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -1424,7 +1424,7 @@ impl Component for EditorView { // If the current document has been changed, apply the changes to all views. // This ensures that selections in jumplists follow changes. if doc.id() == original_doc_id - && doc.get_current_revision() > original_doc_revision + && doc.get_current_revision() != original_doc_revision { if let Some(transaction) = doc.history.get_mut().changes_since(original_doc_revision) diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index 114bf22211a5..95bd95b73a7e 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -283,3 +283,31 @@ async fn test_multi_selection_shell_commands() -> anyhow::Result<()> { Ok(()) } + +#[tokio::test(flavor = "multi_thread")] +async fn test_undo_redo() -> anyhow::Result<()> { + // A jumplist selection is created at a point which is undone. + // + // * 2[ Add two newlines at line start. We're now on line 3. + // * Save the selection on line 3 in the jumplist. + // * u Undo the two newlines. We're now on line 1. + // * Jump forward an back again in the jumplist. This would panic + // if the jumplist were not being updated correctly. + test(("#[|]#", "2[u", "#[|]#")).await?; + + // A jumplist selection is passed through an edit and then an undo and then a redo. + // + // * [ Add a newline at line start. We're now on line 2. + // * Save the selection on line 2 in the jumplist. + // * kd Delete line 1. The jumplist selection should be adjusted to the new line 1. + // * uU Undo and redo the `kd` edit. + // * Jump back in the jumplist. This would panic if the jumplist were not being + // updated correctly. + // * Jump forward to line 1. + test(("#[|]#", "[kduU", "#[|]#")).await?; + + // In this case we 'redo' manually to ensure that the transactions are composing correctly. + test(("#[|]#", "[u[u", "#[|]#")).await?; + + Ok(()) +} diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 087085283391..0eb54f25fa6e 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -857,11 +857,11 @@ impl Document { success } - fn undo_redo_impl(&mut self, view: &mut View, undo: bool) -> bool { + fn undo_redo_impl(&mut self, view_id: ViewId, undo: bool) -> bool { let mut history = self.history.take(); let txn = if undo { history.undo() } else { history.redo() }; let success = if let Some(txn) = txn { - self.apply_impl(txn, view.id) && view.apply(txn, self) + self.apply_impl(txn, view_id) } else { false }; @@ -875,13 +875,13 @@ impl Document { } /// Undo the last modification to the [`Document`]. Returns whether the undo was successful. - pub fn undo(&mut self, view: &mut View) -> bool { - self.undo_redo_impl(view, true) + pub fn undo(&mut self, view_id: ViewId) -> bool { + self.undo_redo_impl(view_id, true) } /// Redo the last modification to the [`Document`]. Returns whether the redo was successful. - pub fn redo(&mut self, view: &mut View) -> bool { - self.undo_redo_impl(view, false) + pub fn redo(&mut self, view_id: ViewId) -> bool { + self.undo_redo_impl(view_id, false) } pub fn savepoint(&mut self) { @@ -894,7 +894,7 @@ impl Document { } } - fn earlier_later_impl(&mut self, view: &mut View, uk: UndoKind, earlier: bool) -> bool { + fn earlier_later_impl(&mut self, view_id: ViewId, uk: UndoKind, earlier: bool) -> bool { let txns = if earlier { self.history.get_mut().earlier(uk) } else { @@ -902,7 +902,7 @@ impl Document { }; let mut success = false; for txn in txns { - if self.apply_impl(&txn, view.id) && view.apply(&txn, self) { + if self.apply_impl(&txn, view_id) { success = true; } } @@ -914,13 +914,13 @@ impl Document { } /// Undo modifications to the [`Document`] according to `uk`. - pub fn earlier(&mut self, view: &mut View, uk: UndoKind) -> bool { - self.earlier_later_impl(view, uk, true) + pub fn earlier(&mut self, view_id: ViewId, uk: UndoKind) -> bool { + self.earlier_later_impl(view_id, uk, true) } /// Redo modifications to the [`Document`] according to `uk`. - pub fn later(&mut self, view: &mut View, uk: UndoKind) -> bool { - self.earlier_later_impl(view, uk, false) + pub fn later(&mut self, view_id: ViewId, uk: UndoKind) -> bool { + self.earlier_later_impl(view_id, uk, false) } /// Commit pending changes to history