aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMichael Davis2022-11-23 03:28:49 +0000
committerGitHub2022-11-23 03:28:49 +0000
commit42e37a571e75aaf4feb1717dfebe8cf215e535dd (patch)
tree385e494b1d10ce6d40b59162dae0b059d4eeb583
parent642a961c032b2a7e7fa67bfc3da54588d0ae8c5b (diff)
Apply transactions to all views (#4733)
* 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>
-rw-r--r--helix-core/src/history.rs18
-rw-r--r--helix-term/src/ui/editor.rs26
-rw-r--r--helix-term/tests/test/splits.rs26
-rw-r--r--helix-view/src/lib.rs8
-rw-r--r--helix-view/src/macros.rs2
-rw-r--r--helix-view/src/view.rs1
6 files changed, 71 insertions, 10 deletions
diff --git a/helix-core/src/history.rs b/helix-core/src/history.rs
index 51174c02..697f29b4 100644
--- a/helix-core/src/history.rs
+++ b/helix-core/src/history.rs
@@ -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>,
@@ -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() {
diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs
index 6c8ee2d9..44f89b77 100644
--- a/helix-term/src/ui/editor.rs
+++ b/helix-term/src/ui/editor.rs
@@ -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() {
@@ -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)
diff --git a/helix-term/tests/test/splits.rs b/helix-term/tests/test/splits.rs
index 5807413a..a51de365 100644
--- a/helix-term/tests/test/splits.rs
+++ b/helix-term/tests/test/splits.rs
@@ -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(())
+}
diff --git a/helix-view/src/lib.rs b/helix-view/src/lib.rs
index 4c32b356..9cf36ae0 100644
--- a/helix-view/src/lib.rs
+++ b/helix-view/src/lib.rs
@@ -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,
) -> bool {
- // This is a short function but it's easy to call `Document::apply`
- // without calling `View::apply` or in the wrong order. The transaction
- // must be applied to the document before the view.
- doc.apply(transaction, view.id) && view.apply(transaction, doc)
+ // TODO remove this helper function. Just call Document::apply everywhere directly.
+ doc.apply(transaction, view.id)
}
pub use document::Document;
diff --git a/helix-view/src/macros.rs b/helix-view/src/macros.rs
index 53ab4346..ee9cd411 100644
--- a/helix-view/src/macros.rs
+++ b/helix-view/src/macros.rs
@@ -67,7 +67,7 @@ macro_rules! view {
#[macro_export]
macro_rules! doc {
($editor:expr, $id:expr) => {{
- $editor.documents[$id]
+ &$editor.documents[$id]
}};
($editor:expr) => {{
$crate::current_ref!($editor).1
diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs
index 8aa4760d..c917a1ab 100644
--- a/helix-view/src/view.rs
+++ b/helix-view/src/view.rs
@@ -351,6 +351,7 @@ impl View {
/// which applies a transaction to the [`Document`] and view together.
pub fn apply(&mut self, transaction: &Transaction, doc: &Document) -> bool {
self.jumps.apply(transaction, doc);
+ // TODO: remove the boolean return. This is unused.
true
}
}