From 4a103db6228ba54e0f36bbebb95d25867458f473 Mon Sep 17 00:00:00 2001
From: Michael Davis <mcarsondavis@gmail.com>
Date: Wed, 23 Nov 2022 09:35:07 -0600
Subject: [PATCH] Apply inversions to Views on undo/redo

When using undo/redo, the history revision can be decremented. In that
case we should apply the inversions since the given revision in
History::changes_since. This prevents panics with jumplist operations
when a session uses undo/redo to move the jumplist selection outside
of the document.
---
 helix-core/src/history.rs         | 24 ++++++++++++++----------
 helix-term/src/ui/editor.rs       |  2 +-
 helix-term/tests/test/commands.rs |  9 +++++++++
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/helix-core/src/history.rs b/helix-core/src/history.rs
index 697f29b4e..5f9fa71e4 100644
--- a/helix-core/src/history.rs
+++ b/helix-core/src/history.rs
@@ -122,17 +122,21 @@ pub const fn at_root(&self) -> bool {
     /// 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<Transaction> {
-        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,
+            Greater => self.revisions[self.current + 1..=revision]
+                .iter()
+                .map(|revision| &revision.inversion)
+                .cloned()
+                .reduce(|acc, inversion| acc.compose(inversion)),
+            Less => self.revisions[revision + 1..=self.current]
+                .iter()
+                .map(|revision| &revision.transaction)
+                .cloned()
+                .reduce(|acc, transaction| acc.compose(transaction)),
+        }
     }
 
     /// Undo the last edit.
diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs
index 44f89b77e..737125031 100644
--- a/helix-term/src/ui/editor.rs
+++ b/helix-term/src/ui/editor.rs
@@ -1424,7 +1424,7 @@ fn handle_event(
                     // 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 27dbd9d7c..012957045 100644
--- a/helix-term/tests/test/commands.rs
+++ b/helix-term/tests/test/commands.rs
@@ -286,6 +286,15 @@ async fn test_multi_selection_shell_commands() -> anyhow::Result<()> {
 
 #[tokio::test(flavor = "multi_thread")]
 async fn test_undo_redo() -> anyhow::Result<()> {
+    // A jumplist selection is created at a point which is undone.
+    //
+    // * 2[<space>   Add two newlines at line start. We're now on line 3.
+    // * <C-s>       Save the selection on line 3 in the jumplist.
+    // * u           Undo the two newlines. We're now on line 1.
+    // * <C-o><C-i>  Jump forward an back again in the jumplist. This would panic
+    //               if the jumplist were not being updated correctly.
+    test(("#[|]#", "2[<space><C-s>u<C-o><C-i>", "#[|]#")).await?;
+
     // A jumplist selection is passed through an edit and then an undo and then a redo.
     //
     // * [<space>    Add a newline at line start. We're now on line 2.