Skip to content

Commit

Permalink
Fix transaction composition order in History::changes_since (helix-ed…
Browse files Browse the repository at this point in the history
…itor#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.
  • Loading branch information
the-mikedavis authored and Frederik Vestre committed Feb 6, 2023
1 parent ecdd513 commit d8e9c75
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 1 deletion.
2 changes: 1 addition & 1 deletion helix-core/src/history.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
25 changes: 25 additions & 0 deletions helix-term/tests/test/splits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}

0 comments on commit d8e9c75

Please sign in to comment.