summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMichael Davis2022-12-03 03:09:08 +0000
committerGitHub2022-12-03 03:09:08 +0000
commitbcdb475b71b0fbabce57344ac8d2575c23b1bbe0 (patch)
tree0714f3a4bf64caee607208b7b3a64409f1ad0f55
parentdc00291b4888f7ffad43fd544259227d0dbbf6fb (diff)
Fix transaction composition order in History::changes_since (#4981)
* Add a undo/redo split test case for crossing branches * history: Switch up/down transaction chaining order The old code tends to work in practice because, usually, either up_txns or down_txns are empty. When both have contents though, we can run into a panic trying to compose them all since they will disagree on the length of the text. This fixes the panic test case in the parent commit.
-rw-r--r--helix-core/src/history.rs2
-rw-r--r--helix-term/tests/test/splits.rs25
2 files changed, 26 insertions, 1 deletions
diff --git a/helix-core/src/history.rs b/helix-core/src/history.rs
index b99e969f..1aac38d9 100644
--- a/helix-core/src/history.rs
+++ b/helix-core/src/history.rs
@@ -131,7 +131,7 @@ impl History {
.map(|&n| self.revisions[n].inversion.clone());
let down_txns = down.iter().map(|&n| self.revisions[n].transaction.clone());
- up_txns.chain(down_txns).reduce(|acc, tx| tx.compose(acc))
+ down_txns.chain(up_txns).reduce(|acc, tx| tx.compose(acc))
}
/// Undo the last edit.
diff --git a/helix-term/tests/test/splits.rs b/helix-term/tests/test/splits.rs
index a34a24b7..96ced21a 100644
--- a/helix-term/tests/test/splits.rs
+++ b/helix-term/tests/test/splits.rs
@@ -161,5 +161,30 @@ async fn test_changes_in_splits_apply_to_all_views() -> anyhow::Result<()> {
))
.await?;
+ // See <https://github.com/helix-editor/helix/issues/4957>.
+ // This sequence undoes part of the history and then adds new changes, creating a
+ // new branch in the history tree. `View::sync_changes` applies transactions down
+ // and up to the lowest common ancestor in the path between old and new revision
+ // numbers. If we apply these up/down transactions in the wrong order, this case
+ // panics.
+ // The key sequence:
+ // * 3[<space> Create three empty lines so we are at the end of the document.
+ // * <C-w>v<C-s> Create a split and save that point at the end of the document
+ // in the jumplist.
+ // * <C-w>w Switch back to the first window.
+ // * uu Undo twice (not three times which would bring us back to the
+ // root of the tree).
+ // * 3[<space> Create three empty lines. Now the end of the document is past
+ // where it was on step 1.
+ // * <C-w>q Close window 1, focusing window 2 and causing a sync. This step
+ // panics if we don't apply in the right order.
+ // * %d Clean up the buffer.
+ test((
+ "#[|]#",
+ "3[<space><C-w>v<C-s><C-w>wuu3[<space><C-w>q%d",
+ "#[|]#",
+ ))
+ .await?;
+
Ok(())
}