Skip to content

Commit

Permalink
Apply transactions to all views (#4733)
Browse files Browse the repository at this point in the history
* Add a test case for updating jumplists across windows

* Apply transactions to all views on history changes

This ensures that jumplist selections follow changes in documents, even
when there are multiple views (for example a split where both windows
edit the same document).

* Leave TODOs for cleaning up View::apply

* Use Iterator::reduce to compose history transactions

Co-authored-by: Blaž Hrastnik <blaz@mxxn.io>

Co-authored-by: Blaž Hrastnik <blaz@mxxn.io>
  • Loading branch information
the-mikedavis and archseer committed Nov 23, 2022
1 parent 642a961 commit 42e37a5
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 10 deletions.
18 changes: 17 additions & 1 deletion helix-core/src/history.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ pub struct History {
}

/// A single point in history. See [History] for more information.
#[derive(Debug)]
#[derive(Debug, Clone)]
struct Revision {
parent: usize,
last_child: Option<NonZeroUsize>,
Expand Down Expand Up @@ -119,6 +119,22 @@ impl History {
self.current == 0
}

/// 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;
}

// 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))
}

/// Undo the last edit.
pub fn undo(&mut self) -> Option<&Transaction> {
if self.at_root() {
Expand Down
26 changes: 23 additions & 3 deletions helix-term/src/ui/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1337,7 +1337,9 @@ impl Component for EditorView {
cx.editor.status_msg = None;

let mode = cx.editor.mode();
let (view, _) = current!(cx.editor);
let (view, doc) = current!(cx.editor);
let original_doc_id = doc.id();
let original_doc_revision = doc.get_current_revision();
let focus = view.id;

if let Some(on_next_key) = self.on_next_key.take() {
Expand Down Expand Up @@ -1413,13 +1415,31 @@ impl Component for EditorView {
let view = view_mut!(cx.editor, focus);
let doc = doc_mut!(cx.editor, &view.doc);

view.ensure_cursor_in_view(doc, config.scrolloff);

// Store a history state if not in insert mode. This also takes care of
// committing changes when leaving insert mode.
if mode != Mode::Insert {
doc.append_changes_to_history(view.id);
}

// 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
{
if let Some(transaction) =
doc.history.get_mut().changes_since(original_doc_revision)
{
let doc = doc!(cx.editor, &original_doc_id);
for (view, _focused) in cx.editor.tree.views_mut() {
view.apply(&transaction, doc);
}
}
}

let view = view_mut!(cx.editor, focus);
let doc = doc_mut!(cx.editor, &view.doc);

view.ensure_cursor_in_view(doc, config.scrolloff);
}

EventResult::Consumed(callback)
Expand Down
26 changes: 26 additions & 0 deletions helix-term/tests/test/splits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,3 +127,29 @@ async fn test_split_write_quit_same_file() -> anyhow::Result<()> {

Ok(())
}

#[tokio::test(flavor = "multi_thread")]
async fn test_changes_in_splits_apply_to_all_views() -> anyhow::Result<()> {
// See <https://github.com/helix-editor/helix/issues/4732>.
// Transactions must be applied to any view that has the changed document open.
// This sequence would panic since the jumplist entry would be modified in one
// window but not the other. Attempting to update the changelist in the other
// window would cause a panic since it would point outside of the document.

// The key sequence here:
// * <C-w>v Create a vertical split of the current buffer.
// Both views look at the same doc.
// * [<space> Add a line ending to the beginning of the document.
// The cursor is now at line 2 in window 2.
// * <C-s> Save that selection to the jumplist in window 2.
// * <C-w>w Switch to window 1.
// * kd Delete line 1 in window 1.
// * <C-w>q Close window 1, focusing window 2.
// * d Delete line 1 in window 2.
//
// This panicked in the past because the jumplist entry on line 2 of window 2
// was not updated and after the `kd` step, pointed outside of the document.
test(("#[|]#", "<C-w>v[<space><C-s><C-w>wkd<C-w>qd", "#[|]#")).await?;

Ok(())
}
8 changes: 3 additions & 5 deletions helix-view/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,10 @@ pub fn align_view(doc: &Document, view: &mut View, align: Align) {
pub fn apply_transaction(
transaction: &helix_core::Transaction,
doc: &mut Document,
view: &mut View,
view: &View,