Apply transactions to all views

Previously, transactions were only applied to the currently focused
view. This could lead to jumplist selections not being updated or
panics if a jumplist selection pointed past the end of the document.

This change moves the application of transactions to the Editor which
can apply the transaction to the document and all views, ensuring that
jumplist entries are updated even if a document is open in multiple
windows.

This complicates most callers of the removed
`helix_view::apply_transaction` helper function. Previously, callers
could take mutable borrows of the view and doc at the beginning of
a function and then pass them to the helper. Now they must take
immutable borrows or refresh the mutable borrows after calling
`Editor::apply_transaction` to avoid taking multiple mutable borrows
of Editor.

A new macro `current_ids` has been added to support a new common case
where we only care about the currently focused document's and view's
IDs.

helix-term/src/commands/lsp.rs

@@ -9,7 +9,7 @@ use tui::text::{Span, Spans};
 use super::{align_view, push_jump, Align, Context, Editor, Open};
 
 use helix_core::{path, Selection};
-use helix_view::{apply_transaction, document::Mode, editor::Action, theme::Style};
+use helix_view::{document::Mode, editor::Action, theme::Style};
 
 use crate::{
     compositor::{self, Compositor},
@@ -711,7 +711,7 @@ pub fn apply_workspace_edit(
             }
         };
 
-        let doc = doc_mut!(editor, &doc_id);
+        let doc = doc!(editor, &doc_id);
 
         // Need to determine a view for apply/append_changes_to_history
         let selections = doc.selections();
@@ -732,7 +732,8 @@ pub fn apply_workspace_edit(
             text_edits,
             offset_encoding,
         );
-        apply_transaction(&transaction, doc, view_mut!(editor, view_id));
+        editor.apply_transaction(&transaction, doc_id, view_id);
+        let doc = doc_mut!(editor, &doc_id);
         doc.append_changes_to_history(view_id);
     };
 

helix-term/src/commands/typed.rs

@@ -4,10 +4,7 @@ use crate::job::Job;
 
 use super::*;
 
-use helix_view::{
-    apply_transaction,
-    editor::{Action, CloseError, ConfigEvent},
-};
+use helix_view::editor::{Action, CloseError, ConfigEvent};
 use ui::completers::{self, Completer};
 
 #[derive(Clone)]
@@ -445,8 +442,8 @@ fn set_line_ending(
         arg if arg.starts_with("nel") => Nel,
         _ => bail!("invalid line ending"),
     };
-    let (view, doc) = current!(cx.editor);
-    doc.line_ending = line_ending;
+    doc_mut!(cx.editor).line_ending = line_ending;
+    let (view, doc) = current_ref!(cx.editor);
 
     let mut pos = 0;
     let transaction = Transaction::change(
@@ -463,7 +460,8 @@ fn set_line_ending(
             }
         }),
     );
-    apply_transaction(&transaction, doc, view);
+    cx.editor.apply_transaction(&transaction, doc.id(), view.id);
+    let (view, doc) = current!(cx.editor);
     doc.append_changes_to_history(view.id);
 
     Ok(())
@@ -480,9 +478,8 @@ fn earlier(
 
     let uk = args.join(" ").parse::<UndoKind>().map_err(|s| anyhow!(s))?;
 
-    let (view, doc) = current!(cx.editor);
-    let success = doc.earlier(view, uk);
-    if !success {
+    let (view_id, doc_id) = current_ids!(cx.editor);
+    if !cx.editor.earlier(doc_id, view_id, uk) {
         cx.editor.set_status("Already at oldest change");
     }
 
@@ -499,9 +496,9 @@ fn later(
     }
 
     let uk = args.join(" ").parse::<UndoKind>().map_err(|s| anyhow!(s))?;
-    let (view, doc) = current!(cx.editor);
-    let success = doc.later(view, uk);
-    if !success {
+
+    let (view_id, doc_id) = current_ids!(cx.editor);
+    if !cx.editor.later(doc_id, view_id, uk) {
         cx.editor.set_status("Already at newest change");
     }
 
@@ -899,7 +896,7 @@ fn replace_selections_with_clipboard_impl(
     cx: &mut compositor::Context,
     clipboard_type: ClipboardType,
 ) -> anyhow::Result<()> {
-    let (view, doc) = current!(cx.editor);
+    let (view, doc) = current_ref!(cx.editor);
 
     match cx.editor.clipboard_provider.get_contents(clipboard_type) {
         Ok(contents) => {
@@ -908,7 +905,8 @@ fn replace_selections_with_clipboard_impl(
                 (range.from(), range.to(), Some(contents.as_str().into()))
             });
 
-            apply_transaction(&transaction, doc, view);
+            cx.editor.apply_transaction(&transaction, doc.id(), view.id);
+            let (view, doc) = current!(cx.editor);
             doc.append_changes_to_history(view.id);
             Ok(())
         }
@@ -1027,11 +1025,8 @@ fn reload(
         return Ok(());
     }
 
-    let scrolloff = cx.editor.config().scrolloff;
-    let (view, doc) = current!(cx.editor);
-    doc.reload(view).map(|_| {
-        view.ensure_cursor_in_view(doc, scrolloff);
-    })
+    let (view_id, doc_id) = current_ids!(cx.editor);
+    cx.editor.reload(doc_id, view_id)
 }
 
 /// Update the [`Document`] if it has been modified.
@@ -1044,8 +1039,7 @@ fn update(
         return Ok(());
     }
 
-    let (_view, doc) = current!(cx.editor);
-    if doc.is_modified() {
+    if doc!(cx.editor).is_modified() {
         write(cx, args, event)
     } else {
         Ok(())
@@ -1061,9 +1055,7 @@ fn lsp_workspace_command(
         return Ok(());
     }
 
-    let (_, doc) = current!(cx.editor);
-
-    let language_server = match doc.language_server() {
+    let language_server = match doc!(cx.editor).language_server() {
         Some(language_server) => language_server,
         None => {
             cx.editor
@@ -1132,7 +1124,8 @@ fn lsp_restart(
         return Ok(());
     }
 
-    let (_view, doc) = current!(cx.editor);
+    let doc = doc!(cx.editor);
+
     let config = doc
         .language_config()
         .context("LSP not defined for the current document")?;
@@ -1166,7 +1159,7 @@ fn tree_sitter_scopes(
         return Ok(());
     }
 
-    let (view, doc) = current!(cx.editor);
+    let (view, doc) = current_ref!(cx.editor);
     let text = doc.text().slice(..);
 
     let pos = doc.selection(view.id).primary().cursor(text);
@@ -1199,10 +1192,10 @@ fn vsplit(
         return Ok(());
     }
 
-    let id = view!(cx.editor).doc;
+    let (_, doc_id) = current_ids!(cx.editor);
 
     if args.is_empty() {
-        cx.editor.switch(id, Action::VerticalSplit);
+        cx.editor.switch(doc_id, Action::VerticalSplit);
     } else {
         for arg in args {
             cx.editor
@@ -1222,10 +1215,10 @@ fn hsplit(
         return Ok(());
     }
 
-    let id = view!(cx.editor).doc;
+    let (_, doc_id) = current_ids!(cx.editor);
 
     if args.is_empty() {
-        cx.editor.switch(id, Action::HorizontalSplit);
+        cx.editor.switch(doc_id, Action::HorizontalSplit);
     } else {
         for arg in args {
             cx.editor
@@ -1504,7 +1497,7 @@ fn sort_impl(
     _args: &[Cow<str>],
     reverse: bool,
 ) -> anyhow::Result<()> {
-    let (view, doc) = current!(cx.editor);
+    let (view, doc) = current_ref!(cx.editor);
     let text = doc.text().slice(..);
 
     let selection = doc.selection(view.id);
@@ -1527,7 +1520,8 @@ fn sort_impl(
             .map(|(s, fragment)| (s.from(), s.to(), Some(fragment))),
     );
 
-    apply_transaction(&transaction, doc, view);
+    cx.editor.apply_transaction(&transaction, doc.id(), view.id);
+    let (view, doc) = current!(cx.editor);
     doc.append_changes_to_history(view.id);
 
     Ok(())
@@ -1543,7 +1537,7 @@ fn reflow(
     }
 
     let scrolloff = cx.editor.config().scrolloff;
-    let (view, doc) = current!(cx.editor);
+    let (view, doc) = current_ref!(cx.editor);
 
     const DEFAULT_MAX_LEN: usize = 79;
 
@@ -1571,7 +1565,8 @@ fn reflow(
         (range.from(), range.to(), Some(reflowed_text))
     });
 
-    apply_transaction(&transaction, doc, view);
+    cx.editor.apply_transaction(&transaction, doc.id(), view.id);
+    let (view, doc) = current!(cx.editor);
     doc.append_changes_to_history(view.id);
     view.ensure_cursor_in_view(doc, scrolloff);
 
@@ -1587,7 +1582,7 @@ fn tree_sitter_subtree(
         return Ok(());
     }
 
-    let (view, doc) = current!(cx.editor);
+    let (view, doc) = current_ref!(cx.editor);
 
     if let Some(syntax) = doc.syntax() {
         let primary_selection = doc.selection(view.id).primary();

helix-term/src/ui/completion.rs

@@ -1,5 +1,5 @@
 use crate::compositor::{Component, Context, Event, EventResult};
-use helix_view::{apply_transaction, editor::CompleteAction};
+use helix_view::editor::CompleteAction;
 use tui::buffer::Buffer as Surface;
 use tui::text::Spans;
 
@@ -148,31 +148,31 @@ impl Completion {
                     .collect()
             }
 
-            let (view, doc) = current!(editor);
+            let (view_id, doc_id) = current_ids!(editor);
 
             // if more text was entered, remove it
-            doc.restore(view);
+            editor.restore(doc_id, view_id);
 
             match event {
                 PromptEvent::Abort => {
-                    doc.restore(view);
+                    editor.restore(doc_id, view_id);
                     editor.last_completion = None;
                 }
                 PromptEvent::Update => {
                     // always present here
                     let item = item.unwrap();
 
                     let transaction = item_to_transaction(
-                        doc,
+                        doc!(editor, &doc_id),
                         item,
                         offset_encoding,
                         start_offset,
                         trigger_offset,
                     );
 
                     // initialize a savepoint
-                    doc.savepoint();
-                    apply_transaction(&transaction, doc, view);
+                    doc_mut!(editor, &doc_id).savepoint();
+                    editor.apply_transaction(&transaction, doc_id, view_id);
 
                     editor.last_completion = Some(CompleteAction {
                         trigger_offset,
@@ -184,14 +184,14 @@ impl Completion {
                     let item = item.unwrap();
 
                     let transaction = item_to_transaction(
-                        doc,
+                        doc!(editor, &doc_id),
                         item,
                         offset_encoding,
                         start_offset,
                         trigger_offset,
                     );
 
-                    apply_transaction(&transaction, doc, view);
+                    editor.apply_transaction(&transaction, doc_id, view_id);
 
                     editor.last_completion = Some(CompleteAction {
                         trigger_offset,
@@ -207,7 +207,7 @@ impl Completion {
                     {
                         None
                     } else {
-                        Self::resolve_completion_item(doc, item.clone())
+                        Self::resolve_completion_item(doc!(editor, &doc_id), item.clone())
                     };
 
                     if let Some(additional_edits) = resolved_item
@@ -216,12 +216,13 @@ impl Completion {
                         .or(item.additional_text_edits.as_ref())
                     {
                         if !additional_edits.is_empty() {
+                            let doc = doc!(editor);
                             let transaction = util::generate_transaction_from_edits(
                                 doc.text(),
                                 additional_edits.clone(),
                                 offset_encoding, // TODO: should probably transcode in Client
                             );
-                            apply_transaction(&transaction, doc, view);
+                            editor.apply_transaction(&transaction, doc_id, view_id);
                         }
                     }
                 }

helix-term/src/ui/editor.rs

@@ -17,7 +17,6 @@ use helix_core::{
     visual_coords_at_pos, LineEnding, Position, Range, Selection, Transaction,
 };
 use helix_view::{
-    apply_transaction,
     document::{Mode, SCRATCH_BUFFER_NAME},
     editor::{CompleteAction, CursorShapeConfig},
     graphics::{Color, CursorKind, Modifier, Rect, Style},
@@ -1018,10 +1017,10 @@ impl EditorView {
                         match key {
                             InsertEvent::Key(key) => self.insert_mode(cxt, key),
                             InsertEvent::CompletionApply(compl) => {
-                                let (view, doc) = current!(cxt.editor);
-
-                                doc.restore(view);
+                                let (view_id, doc_id) = current_ids!(cxt.editor);
+                                cxt.editor.restore(doc_id, view_id);
 
+                                let (view, doc) = current_ref!(cxt.editor);
                                 let text = doc.text().slice(..);
                                 let cursor = doc.selection(view.id).primary().cursor(text);
 
@@ -1034,7 +1033,7 @@ impl EditorView {
                                         (shift_position(start), shift_position(end), t)
                                     }),
                                 );
-                                apply_transaction(&tx, doc, view);
+                                cxt.editor.apply_transaction(&tx, doc_id, view_id);
                             }
                             InsertEvent::TriggerCompletion => {
                                 let (_, doc) = current!(cxt.editor);

helix-term/tests/test/helpers.rs

@@ -1,3 +1,6 @@
+// TODO: remove this when the MSRV is 1.62.0
+#![allow(mutable_borrow_reservation_conflict)]
+
 use std::{
     fs::File,
     io::{Read, Write},
@@ -121,7 +124,7 @@ pub async fn test_key_sequence_with_input_text<T: Into<TestCase>>(
         None => Application::new(Args::default(), Config::default(), test_syntax_conf(None))?,
     };
 
-    let (view, doc) = helix_view::current!(app.editor);
+    let (view, doc) = helix_view::current_ref!(app.editor);
     let sel = doc.selection(view.id).clone();
 
     // replace the initial text with the input text
@@ -130,7 +133,8 @@ pub async fn test_key_sequence_with_input_text<T: Into<TestCase>>(
     })
     .with_selection(test_case.in_selection.clone());
 
-    helix_view::apply_transaction(&transaction, doc, view);
+    app.editor
+        .apply_transaction(&transaction, doc.id(), view.id);
 
     test_key_sequence(
         &mut app,
@@ -307,15 +311,15 @@ impl AppBuilder {
         let mut app = Application::new(self.args, self.config, self.syn_conf)?;
 
         if let Some((text, selection)) = self.input {
-            let (view, doc) = helix_view::current!(app.editor);
+            let (view, doc) = helix_view::current_ref!(app.editor);
             let sel = doc.selection(view.id).clone();
             let trans = Transaction::change_by_selection(doc.text(), &sel, |_| {
                 (0, doc.text().len_chars(), Some((text.clone()).into()))
             })
             .with_selection(selection);
 
             // replace the initial text with the input text
-            helix_view::apply_transaction(&trans, doc, view);
+            app.editor.apply_transaction(&trans, doc.id(), view.id);
         }
 
         Ok(app)