From e039ea29cdfc23998d0a62cc930b5380c5c94ee7 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Mon, 10 Oct 2022 14:54:57 -0500 Subject: [PATCH 1/3] Add View::apply for adjusting jumplist ranges Applying a transaction to a View adjusts the ranges in the jumplist to ensure that they remain within the text of the document and follow regular selection invariants (for example, must be have a width of at least one). --- helix-view/src/view.rs | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs index 3df533dfc6d9..62984b889476 100644 --- a/helix-view/src/view.rs +++ b/helix-view/src/view.rs @@ -3,7 +3,9 @@ use crate::{ gutter::{self, Gutter}, Document, DocumentId, ViewId, }; -use helix_core::{pos_at_visual_coords, visual_coords_at_pos, Position, RopeSlice, Selection}; +use helix_core::{ + pos_at_visual_coords, visual_coords_at_pos, Position, RopeSlice, Selection, Transaction, +}; use std::fmt; @@ -62,6 +64,22 @@ impl JumpList { pub fn get(&self) -> &[Jump] { &self.jumps } + + /// Applies a [`Transaction`] of changes to the jumplist. + /// This is necessary to ensure that changes to documents do not leave jump-list + /// selections pointing to parts of the text which no longer exist. + fn apply(&mut self, transaction: &Transaction, doc: &Document) { + let text = doc.text().slice(..); + + for (doc_id, selection) in &mut self.jumps { + if doc.id() == *doc_id { + *selection = selection + .clone() + .map(transaction.changes()) + .ensure_invariants(text); + } + } + } } #[derive(Clone)] @@ -334,6 +352,14 @@ impl View { // (None, None) => return, // } // } + + /// Applies a [`Transaction`] to the view. + /// Instead of calling this function directly, use [crate::apply_transaction] + /// 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); + true + } } #[cfg(test)] From 9f97d47ab729545660f4a6598320e115796b0d7f Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Mon, 10 Oct 2022 14:56:26 -0500 Subject: [PATCH 2/3] Apply transactions to Views This change adds View::apply calls for all Document::apply call-sites, ensuring that changes to a document do not leave invalid entries in the View's jumplist. --- helix-term/src/commands.rs | 43 ++++++++++++++++++++++++++------ helix-term/src/commands/lsp.rs | 5 ++-- helix-term/src/commands/typed.rs | 6 ++++- helix-term/src/ui/completion.rs | 7 ++++-- helix-term/src/ui/editor.rs | 3 ++- helix-view/src/document.rs | 14 ++++++----- 6 files changed, 58 insertions(+), 20 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 2db5bfcf3019..e2bd9402d95b 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -860,6 +860,7 @@ fn align_selections(cx: &mut Context) { let transaction = Transaction::change(doc.text(), changes.into_iter()); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); } fn goto_window(cx: &mut Context, align: Align) { @@ -1290,6 +1291,7 @@ fn replace(cx: &mut Context) { }); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); } }) } @@ -1307,6 +1309,7 @@ where }); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); } fn switch_case(cx: &mut Context) { @@ -2113,6 +2116,7 @@ fn delete_selection_impl(cx: &mut Context, op: Operation) { (range.from(), range.to(), None) }); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); match op { Operation::Delete => { @@ -2126,7 +2130,7 @@ fn delete_selection_impl(cx: &mut Context, op: Operation) { } #[inline] -fn delete_selection_insert_mode(doc: &mut Document, view: &View, selection: &Selection) { +fn delete_selection_insert_mode(doc: &mut Document, view: &mut View, selection: &Selection) { let view_id = view.id; // then delete @@ -2134,6 +2138,7 @@ fn delete_selection_insert_mode(doc: &mut Document, view: &View, selection: &Sel (range.from(), range.to(), None) }); doc.apply(&transaction, view_id); + view.apply(&transaction, doc); } fn delete_selection(cx: &mut Context) { @@ -2230,6 +2235,7 @@ fn append_mode(cx: &mut Context) { [(end, end, Some(doc.line_ending.as_str().into()))].into_iter(), ); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); } let selection = doc.selection(view.id).clone().transform(|range| { @@ -2530,6 +2536,7 @@ async fn make_format_callback( let view = view_mut!(editor); if doc.version() == doc_version { doc.apply(&format, view.id); + view.apply(&format, doc); doc.append_changes_to_history(view.id); doc.detect_indent_and_line_ending(); view.ensure_cursor_in_view(doc, scrolloff); @@ -2617,6 +2624,7 @@ fn open(cx: &mut Context, open: Open) { transaction = transaction.with_selection(Selection::new(ranges, selection.primary_index())); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); } // o inserts a new line after each line with a selection @@ -2637,7 +2645,7 @@ fn normal_mode(cx: &mut Context) { cx.editor.mode = Mode::Normal; let (view, doc) = current!(cx.editor); - try_restore_indent(doc, view.id); + try_restore_indent(doc, view); // if leaving append mode, move cursor back by 1 if doc.restore_cursor { @@ -2654,7 +2662,7 @@ fn normal_mode(cx: &mut Context) { } } -fn try_restore_indent(doc: &mut Document, view_id: ViewId) { +fn try_restore_indent(doc: &mut Document, view: &mut View) { use helix_core::chars::char_is_whitespace; use helix_core::Operation; @@ -2673,18 +2681,19 @@ fn try_restore_indent(doc: &mut Document, view_id: ViewId) { let doc_changes = doc.changes().changes(); let text = doc.text().slice(..); - let range = doc.selection(view_id).primary(); + let range = doc.selection(view.id).primary(); let pos = range.cursor(text); let line_end_pos = line_end_char_index(&text, range.cursor_line(text)); if inserted_a_new_blank_line(doc_changes, pos, line_end_pos) { // Removes tailing whitespaces. let transaction = - Transaction::change_by_selection(doc.text(), doc.selection(view_id), |range| { + Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { let line_start_pos = text.line_to_char(range.cursor_line(text)); (line_start_pos, pos, None) }); - doc.apply(&transaction, view_id); + doc.apply(&transaction, view.id); + view.apply(&transaction, doc); } } @@ -2999,6 +3008,7 @@ pub mod insert { let (view, doc) = current!(cx.editor); if let Some(t) = transaction { doc.apply(&t, view.id); + view.apply(&t, doc); } // TODO: need a post insert hook too for certain triggers (autocomplete, signature help, etc) @@ -3021,6 +3031,7 @@ pub mod insert { indent, ); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); } pub fn insert_newline(cx: &mut Context) { @@ -3108,6 +3119,7 @@ pub mod insert { let (view, doc) = current!(cx.editor); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); } pub fn delete_char_backward(cx: &mut Context) { @@ -3202,6 +3214,7 @@ pub mod insert { }); let (view, doc) = current!(cx.editor); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); lsp::signature_help_impl(cx, SignatureHelpInvoked::Automatic); } @@ -3220,6 +3233,7 @@ pub mod insert { ) }); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); lsp::signature_help_impl(cx, SignatureHelpInvoked::Automatic); } @@ -3413,7 +3427,7 @@ enum Paste { Cursor, } -fn paste_impl(values: &[String], doc: &mut Document, view: &View, action: Paste, count: usize) { +fn paste_impl(values: &[String], doc: &mut Document, view: &mut View, action: Paste, count: usize) { let repeat = std::iter::repeat( values .last() @@ -3457,6 +3471,7 @@ fn paste_impl(values: &[String], doc: &mut Document, view: &View, action: Paste, (pos, pos, values.next()) }); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); } pub(crate) fn paste_bracketed_value(cx: &mut Context, contents: String) { @@ -3549,6 +3564,7 @@ fn replace_with_yanked(cx: &mut Context) { }); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); } } } @@ -3572,6 +3588,7 @@ fn replace_selections_with_clipboard_impl( }); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); doc.append_changes_to_history(view.id); Ok(()) } @@ -3642,6 +3659,7 @@ fn indent(cx: &mut Context) { }), ); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); } fn unindent(cx: &mut Context) { @@ -3681,6 +3699,7 @@ fn unindent(cx: &mut Context) { let transaction = Transaction::change(doc.text(), changes.into_iter()); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); } fn format_selections(cx: &mut Context) { @@ -3728,6 +3747,7 @@ fn format_selections(cx: &mut Context) { // ); // doc.apply(&transaction, view.id); + // view.apply(&transaction, doc); } } @@ -3783,6 +3803,7 @@ fn join_selections_inner(cx: &mut Context, select_space: bool) { }; doc.apply(&transaction, view.id); + view.apply(&transaction, doc); } fn keep_or_remove_selections_impl(cx: &mut Context, remove: bool) { @@ -3936,6 +3957,7 @@ fn toggle_comments(cx: &mut Context) { let transaction = comment::toggle_line_comments(doc.text(), doc.selection(view.id), token); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); exit_select_mode(cx); } @@ -3992,6 +4014,7 @@ fn rotate_selection_contents(cx: &mut Context, direction: Direction) { ); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); } fn rotate_selection_contents_forward(cx: &mut Context) { @@ -4488,6 +4511,7 @@ fn surround_add(cx: &mut Context) { let transaction = Transaction::change(doc.text(), changes.into_iter()); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); }) } @@ -4527,6 +4551,7 @@ fn surround_replace(cx: &mut Context) { }), ); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); }); }) } @@ -4554,6 +4579,7 @@ fn surround_delete(cx: &mut Context) { let transaction = Transaction::change(doc.text(), change_pos.into_iter().map(|p| (p, p + 1, None))); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); }) } @@ -4729,6 +4755,7 @@ fn shell(cx: &mut compositor::Context, cmd: &str, behavior: &ShellBehavior) { if behavior != &ShellBehavior::Ignore { let transaction = Transaction::change(doc.text(), changes.into_iter()); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); doc.append_changes_to_history(view.id); } @@ -4792,6 +4819,7 @@ fn add_newline_impl(cx: &mut Context, open: Open) { let transaction = Transaction::change(text, changes); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); } /// Increment object under cursor by count. @@ -4885,6 +4913,7 @@ fn increment_impl(cx: &mut Context, amount: i64) { let transaction = transaction.with_selection(selection.clone()); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); } } diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 1113b44ecdf7..726aec674d64 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -596,9 +596,7 @@ pub fn apply_workspace_edit( } }; - let doc = editor - .document_mut(doc_id) - .expect("Document for document_changes not found"); + let doc = doc_mut!(editor, &doc_id); // Need to determine a view for apply/append_changes_to_history let selections = doc.selections(); @@ -620,6 +618,7 @@ pub fn apply_workspace_edit( offset_encoding, ); doc.apply(&transaction, view_id); + view_mut!(editor, view_id).apply(&transaction, doc); doc.append_changes_to_history(view_id); }; diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 96ff75c54a0f..574e895b56b5 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -463,6 +463,7 @@ fn set_line_ending( }), ); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); doc.append_changes_to_history(view.id); Ok(()) @@ -884,6 +885,7 @@ fn replace_selections_with_clipboard_impl( }); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); doc.append_changes_to_history(view.id); Ok(()) } @@ -1004,7 +1006,7 @@ fn reload( let scrolloff = cx.editor.config().scrolloff; let (view, doc) = current!(cx.editor); - doc.reload(view.id).map(|_| { + doc.reload(view).map(|_| { view.ensure_cursor_in_view(doc, scrolloff); }) } @@ -1399,6 +1401,7 @@ fn sort_impl( ); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); doc.append_changes_to_history(view.id); Ok(()) @@ -1443,6 +1446,7 @@ fn reflow( }); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); doc.append_changes_to_history(view.id); view.ensure_cursor_in_view(doc, scrolloff); diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs index 2d7d4f925695..c0a5da2e5175 100644 --- a/helix-term/src/ui/completion.rs +++ b/helix-term/src/ui/completion.rs @@ -143,11 +143,11 @@ impl Completion { let (view, doc) = current!(editor); // if more text was entered, remove it - doc.restore(view.id); + doc.restore(view); match event { PromptEvent::Abort => { - doc.restore(view.id); + doc.restore(view); editor.last_completion = None; } PromptEvent::Update => { @@ -165,6 +165,7 @@ impl Completion { // initialize a savepoint doc.savepoint(); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); editor.last_completion = Some(CompleteAction { trigger_offset, @@ -184,6 +185,7 @@ impl Completion { ); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); editor.last_completion = Some(CompleteAction { trigger_offset, @@ -214,6 +216,7 @@ impl Completion { offset_encoding, // TODO: should probably transcode in Client ); doc.apply(&transaction, view.id); + view.apply(&transaction, doc); } } } diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index 47fb7a4a5408..dfac66dafe96 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -988,7 +988,7 @@ impl EditorView { InsertEvent::CompletionApply(compl) => { let (view, doc) = current!(cxt.editor); - doc.restore(view.id); + doc.restore(view); let text = doc.text().slice(..); let cursor = doc.selection(view.id).primary().cursor(text); @@ -1003,6 +1003,7 @@ impl EditorView { }), ); doc.apply(&tx, view.id); + view.apply(&tx, doc); } InsertEvent::TriggerCompletion => { let (_, doc) = current!(cxt.editor); diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index b6b2f664c1a8..631c540b094c 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -24,7 +24,7 @@ use helix_core::{ DEFAULT_LINE_ENDING, }; -use crate::{DocumentId, Editor, ViewId}; +use crate::{DocumentId, Editor, View, ViewId}; /// 8kB of buffer space for encoding and decoding `Rope`s. const BUF_SIZE: usize = 8192; @@ -601,7 +601,7 @@ impl Document { } /// Reload the document from its path. - pub fn reload(&mut self, view_id: ViewId) -> Result<(), Error> { + pub fn reload(&mut self, view: &mut View) -> Result<(), Error> { let encoding = &self.encoding; let path = self.path().filter(|path| path.exists()); @@ -617,8 +617,9 @@ impl Document { // This is not considered a modification of the contents of the file regardless // of the encoding. let transaction = helix_core::diff::compare_ropes(self.text(), &rope); - self.apply(&transaction, view_id); - self.append_changes_to_history(view_id); + self.apply(&transaction, view.id); + view.apply(&transaction, self); + self.append_changes_to_history(view.id); self.reset_modified(); self.detect_indent_and_line_ending(); @@ -862,9 +863,10 @@ impl Document { self.savepoint = Some(Transaction::new(self.text())); } - pub fn restore(&mut self, view_id: ViewId) { + pub fn restore(&mut self, view: &mut View) { if let Some(revert) = self.savepoint.take() { - self.apply(&revert, view_id); + self.apply(&revert, view.id); + view.apply(&revert, self); } } From 9a763c6ecb662523299de94614a2e114a62eede6 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Mon, 10 Oct 2022 15:15:37 -0500 Subject: [PATCH 3/3] Add a helper function for applying transactions It is easy to forget to call `Document::apply` and/or `View::apply` in the correct order. This commit introduces a helper function which closes over both calls. --- helix-term/src/commands.rs | 91 +++++++++++--------------------- helix-term/src/commands/lsp.rs | 5 +- helix-term/src/commands/typed.rs | 17 +++--- helix-term/src/ui/completion.rs | 11 ++-- helix-term/src/ui/editor.rs | 4 +- helix-view/src/document.rs | 11 ++-- helix-view/src/lib.rs | 13 +++++ 7 files changed, 65 insertions(+), 87 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index e2bd9402d95b..754e4473163e 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -27,6 +27,7 @@ use helix_core::{ SmallVec, Tendril, Transaction, }; use helix_view::{ + apply_transaction, clipboard::ClipboardType, document::{FormatterError, Mode, SCRATCH_BUFFER_NAME}, editor::{Action, Motion}, @@ -859,8 +860,7 @@ fn align_selections(cx: &mut Context) { changes.sort_unstable_by_key(|(from, _, _)| *from); let transaction = Transaction::change(doc.text(), changes.into_iter()); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); } fn goto_window(cx: &mut Context, align: Align) { @@ -1290,8 +1290,7 @@ fn replace(cx: &mut Context) { } }); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); } }) } @@ -1308,8 +1307,7 @@ where (range.from(), range.to(), Some(text)) }); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); } fn switch_case(cx: &mut Context) { @@ -2115,8 +2113,7 @@ fn delete_selection_impl(cx: &mut Context, op: Operation) { let transaction = Transaction::change_by_selection(doc.text(), selection, |range| { (range.from(), range.to(), None) }); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); match op { Operation::Delete => { @@ -2131,14 +2128,10 @@ fn delete_selection_impl(cx: &mut Context, op: Operation) { #[inline] fn delete_selection_insert_mode(doc: &mut Document, view: &mut View, selection: &Selection) { - let view_id = view.id; - - // then delete let transaction = Transaction::change_by_selection(doc.text(), selection, |range| { (range.from(), range.to(), None) }); - doc.apply(&transaction, view_id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); } fn delete_selection(cx: &mut Context) { @@ -2234,8 +2227,7 @@ fn append_mode(cx: &mut Context) { doc.text(), [(end, end, Some(doc.line_ending.as_str().into()))].into_iter(), ); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); } let selection = doc.selection(view.id).clone().transform(|range| { @@ -2535,8 +2527,7 @@ async fn make_format_callback( let doc = doc_mut!(editor, &doc_id); let view = view_mut!(editor); if doc.version() == doc_version { - doc.apply(&format, view.id); - view.apply(&format, doc); + apply_transaction(&format, doc, view); doc.append_changes_to_history(view.id); doc.detect_indent_and_line_ending(); view.ensure_cursor_in_view(doc, scrolloff); @@ -2623,8 +2614,7 @@ fn open(cx: &mut Context, open: Open) { transaction = transaction.with_selection(Selection::new(ranges, selection.primary_index())); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); } // o inserts a new line after each line with a selection @@ -2692,8 +2682,7 @@ fn try_restore_indent(doc: &mut Document, view: &mut View) { let line_start_pos = text.line_to_char(range.cursor_line(text)); (line_start_pos, pos, None) }); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); } } @@ -3007,8 +2996,7 @@ pub mod insert { let (view, doc) = current!(cx.editor); if let Some(t) = transaction { - doc.apply(&t, view.id); - view.apply(&t, doc); + apply_transaction(&t, doc, view); } // TODO: need a post insert hook too for certain triggers (autocomplete, signature help, etc) @@ -3030,8 +3018,7 @@ pub mod insert { &doc.selection(view.id).clone().cursors(doc.text().slice(..)), indent, ); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); } pub fn insert_newline(cx: &mut Context) { @@ -3118,8 +3105,7 @@ pub mod insert { transaction = transaction.with_selection(Selection::new(ranges, selection.primary_index())); let (view, doc) = current!(cx.editor); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); } pub fn delete_char_backward(cx: &mut Context) { @@ -3213,8 +3199,7 @@ pub mod insert { } }); let (view, doc) = current!(cx.editor); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); lsp::signature_help_impl(cx, SignatureHelpInvoked::Automatic); } @@ -3232,8 +3217,7 @@ pub mod insert { None, ) }); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); lsp::signature_help_impl(cx, SignatureHelpInvoked::Automatic); } @@ -3470,8 +3454,7 @@ fn paste_impl(values: &[String], doc: &mut Document, view: &mut View, action: Pa }; (pos, pos, values.next()) }); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); } pub(crate) fn paste_bracketed_value(cx: &mut Context, contents: String) { @@ -3563,8 +3546,7 @@ fn replace_with_yanked(cx: &mut Context) { } }); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); } } } @@ -3587,8 +3569,7 @@ fn replace_selections_with_clipboard_impl( ) }); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); doc.append_changes_to_history(view.id); Ok(()) } @@ -3658,8 +3639,7 @@ fn indent(cx: &mut Context) { Some((pos, pos, Some(indent.clone()))) }), ); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); } fn unindent(cx: &mut Context) { @@ -3698,8 +3678,7 @@ fn unindent(cx: &mut Context) { let transaction = Transaction::change(doc.text(), changes.into_iter()); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); } fn format_selections(cx: &mut Context) { @@ -3746,8 +3725,7 @@ fn format_selections(cx: &mut Context) { // language_server.offset_encoding(), // ); - // doc.apply(&transaction, view.id); - // view.apply(&transaction, doc); + // apply_transaction(&transaction, doc, view); } } @@ -3802,8 +3780,7 @@ fn join_selections_inner(cx: &mut Context, select_space: bool) { Transaction::change(doc.text(), changes.into_iter()) }; - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); } fn keep_or_remove_selections_impl(cx: &mut Context, remove: bool) { @@ -3956,8 +3933,7 @@ fn toggle_comments(cx: &mut Context) { .map(|tc| tc.as_ref()); let transaction = comment::toggle_line_comments(doc.text(), doc.selection(view.id), token); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); exit_select_mode(cx); } @@ -4013,8 +3989,7 @@ fn rotate_selection_contents(cx: &mut Context, direction: Direction) { .map(|(range, fragment)| (range.from(), range.to(), Some(fragment))), ); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); } fn rotate_selection_contents_forward(cx: &mut Context) { @@ -4510,8 +4485,7 @@ fn surround_add(cx: &mut Context) { } let transaction = Transaction::change(doc.text(), changes.into_iter()); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); }) } @@ -4550,8 +4524,7 @@ fn surround_replace(cx: &mut Context) { (pos, pos + 1, Some(t)) }), ); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); }); }) } @@ -4578,8 +4551,7 @@ fn surround_delete(cx: &mut Context) { let transaction = Transaction::change(doc.text(), change_pos.into_iter().map(|p| (p, p + 1, None))); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); }) } @@ -4754,8 +4726,7 @@ fn shell(cx: &mut compositor::Context, cmd: &str, behavior: &ShellBehavior) { if behavior != &ShellBehavior::Ignore { let transaction = Transaction::change(doc.text(), changes.into_iter()); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); doc.append_changes_to_history(view.id); } @@ -4818,8 +4789,7 @@ fn add_newline_impl(cx: &mut Context, open: Open) { }); let transaction = Transaction::change(text, changes); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); } /// Increment object under cursor by count. @@ -4912,8 +4882,7 @@ fn increment_impl(cx: &mut Context, amount: i64) { let transaction = Transaction::change(doc.text(), changes); let transaction = transaction.with_selection(selection.clone()); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); } } diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 726aec674d64..3fa5c96fff83 100644 --- a/helix-term/src/commands/lsp.rs +++ b/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::{editor::Action, theme::Style}; +use helix_view::{apply_transaction, editor::Action, theme::Style}; use crate::{ compositor::{self, Compositor}, @@ -617,8 +617,7 @@ pub fn apply_workspace_edit( text_edits, offset_encoding, ); - doc.apply(&transaction, view_id); - view_mut!(editor, view_id).apply(&transaction, doc); + apply_transaction(&transaction, doc, view_mut!(editor, view_id)); doc.append_changes_to_history(view_id); }; diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 574e895b56b5..726c745669d5 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -2,7 +2,10 @@ use std::ops::Deref; use super::*; -use helix_view::editor::{Action, CloseError, ConfigEvent}; +use helix_view::{ + apply_transaction, + editor::{Action, CloseError, ConfigEvent}, +}; use ui::completers::{self, Completer}; #[derive(Clone)] @@ -462,8 +465,7 @@ fn set_line_ending( } }), ); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); doc.append_changes_to_history(view.id); Ok(()) @@ -884,8 +886,7 @@ fn replace_selections_with_clipboard_impl( (range.from(), range.to(), Some(contents.as_str().into())) }); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); doc.append_changes_to_history(view.id); Ok(()) } @@ -1400,8 +1401,7 @@ fn sort_impl( .map(|(s, fragment)| (s.from(), s.to(), Some(fragment))), ); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); doc.append_changes_to_history(view.id); Ok(()) @@ -1445,8 +1445,7 @@ fn reflow( (range.from(), range.to(), Some(reflowed_text)) }); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); doc.append_changes_to_history(view.id); view.ensure_cursor_in_view(doc, scrolloff); diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs index c0a5da2e5175..7348dcf44431 100644 --- a/helix-term/src/ui/completion.rs +++ b/helix-term/src/ui/completion.rs @@ -1,5 +1,5 @@ use crate::compositor::{Component, Context, Event, EventResult}; -use helix_view::editor::CompleteAction; +use helix_view::{apply_transaction, editor::CompleteAction}; use tui::buffer::Buffer as Surface; use tui::text::Spans; @@ -164,8 +164,7 @@ impl Completion { // initialize a savepoint doc.savepoint(); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); editor.last_completion = Some(CompleteAction { trigger_offset, @@ -184,8 +183,7 @@ impl Completion { trigger_offset, ); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); editor.last_completion = Some(CompleteAction { trigger_offset, @@ -215,8 +213,7 @@ impl Completion { additional_edits.clone(), offset_encoding, // TODO: should probably transcode in Client ); - doc.apply(&transaction, view.id); - view.apply(&transaction, doc); + apply_transaction(&transaction, doc, view); } } } diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index dfac66dafe96..263babd72e77 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -16,6 +16,7 @@ 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}, @@ -1002,8 +1003,7 @@ impl EditorView { (shift_position(start), shift_position(end), t) }), ); - doc.apply(&tx, view.id); - view.apply(&tx, doc); + apply_transaction(&tx, doc, view); } InsertEvent::TriggerCompletion => { let (_, doc) = current!(cxt.editor); diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 631c540b094c..9370833999d3 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -24,7 +24,7 @@ use helix_core::{ DEFAULT_LINE_ENDING, }; -use crate::{DocumentId, Editor, View, ViewId}; +use crate::{apply_transaction, DocumentId, Editor, View, ViewId}; /// 8kB of buffer space for encoding and decoding `Rope`s. const BUF_SIZE: usize = 8192; @@ -617,8 +617,7 @@ impl Document { // This is not considered a modification of the contents of the file regardless // of the encoding. let transaction = helix_core::diff::compare_ropes(self.text(), &rope); - self.apply(&transaction, view.id); - view.apply(&transaction, self); + apply_transaction(&transaction, self, view); self.append_changes_to_history(view.id); self.reset_modified(); @@ -811,6 +810,9 @@ impl Document { } /// Apply a [`Transaction`] to the [`Document`] to change its text. + /// Instead of calling this function directly, use [crate::apply_transaction] + /// to ensure that the transaction is applied to the appropriate [`View`] as + /// well. pub fn apply(&mut self, transaction: &Transaction, view_id: ViewId) -> bool { // store the state just before any changes are made. This allows us to undo to the // state just before a transaction was applied. @@ -865,8 +867,7 @@ impl Document { pub fn restore(&mut self, view: &mut View) { if let Some(revert) = self.savepoint.take() { - self.apply(&revert, view.id); - view.apply(&revert, self); + apply_transaction(&revert, self, view); } } diff --git a/helix-view/src/lib.rs b/helix-view/src/lib.rs index 0145677dc8bc..276be44186a7 100644 --- a/helix-view/src/lib.rs +++ b/helix-view/src/lib.rs @@ -64,6 +64,19 @@ pub fn align_view(doc: &Document, view: &mut View, align: Align) { view.offset.row = line.saturating_sub(relative); } +/// Applies a [`helix_core::Transaction`] to the given [`Document`] +/// and [`View`]. +pub fn apply_transaction( + transaction: &helix_core::Transaction, + doc: &mut Document, + view: &mut 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) +} + pub use document::Document; pub use editor::Editor; pub use theme::Theme;