From a20a96abdcfbea3b3eb890bfc266850938434a90 Mon Sep 17 00:00:00 2001 From: Ivan Tham Date: Sun, 22 Jan 2023 02:13:43 +0800 Subject: [PATCH] Remove apply_transaction helper (#5598) --- helix-term/src/commands.rs | 57 ++++++++++++++++---------------- helix-term/src/commands/lsp.rs | 4 +-- helix-term/src/commands/typed.rs | 13 +++----- helix-term/src/ui/completion.rs | 8 ++--- helix-term/src/ui/editor.rs | 3 +- helix-term/tests/test/helpers.rs | 4 +-- helix-view/src/document.rs | 9 ++--- helix-view/src/editor.rs | 2 +- helix-view/src/lib.rs | 11 ------ helix-view/src/view.rs | 2 -- 10 files changed, 46 insertions(+), 67 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 8b742d00a..e773a2c78 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -26,7 +26,6 @@ SmallVec, Tendril, Transaction, }; use helix_view::{ - apply_transaction, clipboard::ClipboardType, document::{FormatterError, Mode, SCRATCH_BUFFER_NAME}, editor::{Action, Motion}, @@ -864,7 +863,7 @@ fn align_selections(cx: &mut Context) { changes.sort_unstable_by_key(|(from, _, _)| *from); let transaction = Transaction::change(doc.text(), changes.into_iter()); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); } fn goto_window(cx: &mut Context, align: Align) { @@ -1316,7 +1315,7 @@ fn replace(cx: &mut Context) { } }); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); exit_select_mode(cx); } }) @@ -1334,7 +1333,7 @@ fn switch_case_impl(cx: &mut Context, change_fn: F) (range.from(), range.to(), Some(text)) }); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); } fn switch_case(cx: &mut Context) { @@ -2159,7 +2158,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) }); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); match op { Operation::Delete => { @@ -2177,7 +2176,7 @@ fn delete_selection_insert_mode(doc: &mut Document, view: &mut View, selection: let transaction = Transaction::change_by_selection(doc.text(), selection, |range| { (range.from(), range.to(), None) }); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); } fn delete_selection(cx: &mut Context) { @@ -2273,7 +2272,7 @@ fn append_mode(cx: &mut Context) { doc.text(), [(end, end, Some(doc.line_ending.as_str().into()))].into_iter(), ); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); } let selection = doc.selection(view.id).clone().transform(|range| { @@ -2583,7 +2582,7 @@ async fn make_format_callback( if let Ok(format) = format { if doc.version() == doc_version { - apply_transaction(&format, doc, view); + doc.apply(&format, view.id); doc.append_changes_to_history(view); doc.detect_indent_and_line_ending(); view.ensure_cursor_in_view(doc, scrolloff); @@ -2676,7 +2675,7 @@ fn open(cx: &mut Context, open: Open) { transaction = transaction.with_selection(Selection::new(ranges, selection.primary_index())); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); } // o inserts a new line after each line with a selection @@ -3104,7 +3103,7 @@ pub fn insert_char(cx: &mut Context, c: char) { let (view, doc) = current!(cx.editor); if let Some(t) = transaction { - apply_transaction(&t, doc, view); + doc.apply(&t, view.id); } // TODO: need a post insert hook too for certain triggers (autocomplete, signature help, etc) @@ -3126,7 +3125,7 @@ pub fn insert_tab(cx: &mut Context) { &doc.selection(view.id).clone().cursors(doc.text().slice(..)), indent, ); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); } pub fn insert_newline(cx: &mut Context) { @@ -3231,7 +3230,7 @@ pub fn insert_newline(cx: &mut Context) { transaction = transaction.with_selection(Selection::new(ranges, selection.primary_index())); let (view, doc) = current!(cx.editor); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); } pub fn delete_char_backward(cx: &mut Context) { @@ -3326,7 +3325,7 @@ pub fn delete_char_backward(cx: &mut Context) { } }); let (view, doc) = current!(cx.editor); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); lsp::signature_help_impl(cx, SignatureHelpInvoked::Automatic); } @@ -3344,7 +3343,7 @@ pub fn delete_char_forward(cx: &mut Context) { None, ) }); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); lsp::signature_help_impl(cx, SignatureHelpInvoked::Automatic); } @@ -3625,7 +3624,7 @@ fn paste_impl( transaction = transaction.with_selection(Selection::new(ranges, selection.primary_index())); } - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); } pub(crate) fn paste_bracketed_value(cx: &mut Context, contents: String) { @@ -3717,7 +3716,7 @@ fn replace_with_yanked(cx: &mut Context) { } }); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); exit_select_mode(cx); } } @@ -3741,7 +3740,7 @@ fn replace_selections_with_clipboard_impl( ) }); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); doc.append_changes_to_history(view); } Err(e) => return Err(e.context("Couldn't get system clipboard contents")), @@ -3813,7 +3812,7 @@ fn indent(cx: &mut Context) { Some((pos, pos, Some(indent.clone()))) }), ); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); } fn unindent(cx: &mut Context) { @@ -3852,7 +3851,7 @@ fn unindent(cx: &mut Context) { let transaction = Transaction::change(doc.text(), changes.into_iter()); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); } fn format_selections(cx: &mut Context) { @@ -3907,7 +3906,7 @@ fn format_selections(cx: &mut Context) { language_server.offset_encoding(), ); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); } fn join_selections_impl(cx: &mut Context, select_space: bool) { @@ -3966,7 +3965,7 @@ fn join_selections_impl(cx: &mut Context, select_space: bool) { Transaction::change(doc.text(), changes.into_iter()) }; - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); } fn keep_or_remove_selections_impl(cx: &mut Context, remove: bool) { @@ -4109,7 +4108,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); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); exit_select_mode(cx); } @@ -4165,7 +4164,7 @@ fn rotate_selection_contents(cx: &mut Context, direction: Direction) { .map(|(range, fragment)| (range.from(), range.to(), Some(fragment))), ); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); } fn rotate_selection_contents_forward(cx: &mut Context) { @@ -4698,7 +4697,7 @@ fn surround_add(cx: &mut Context) { let transaction = Transaction::change(doc.text(), changes.into_iter()) .with_selection(Selection::new(ranges, selection.primary_index())); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); exit_select_mode(cx); }) } @@ -4738,7 +4737,7 @@ fn surround_replace(cx: &mut Context) { (pos, pos + 1, Some(t)) }), ); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); exit_select_mode(cx); }); }) @@ -4766,7 +4765,7 @@ fn surround_delete(cx: &mut Context) { let transaction = Transaction::change(doc.text(), change_pos.into_iter().map(|p| (p, p + 1, None))); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); exit_select_mode(cx); }) } @@ -4981,7 +4980,7 @@ fn shell(cx: &mut compositor::Context, cmd: &str, behavior: &ShellBehavior) { if behavior != &ShellBehavior::Ignore { let transaction = Transaction::change(doc.text(), changes.into_iter()) .with_selection(Selection::new(ranges, selection.primary_index())); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); doc.append_changes_to_history(view); } @@ -5044,7 +5043,7 @@ fn add_newline_impl(cx: &mut Context, open: Open) { }); let transaction = Transaction::change(text, changes); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); } enum IncrementDirection { @@ -5111,7 +5110,7 @@ fn increment_impl(cx: &mut Context, increment_direction: IncrementDirection) { let new_selection = Selection::new(new_selection_ranges, selection.primary_index()); let transaction = Transaction::change(doc.text(), changes.into_iter()); let transaction = transaction.with_selection(new_selection); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); } } diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 90b6d76c0..578eb6084 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -13,7 +13,7 @@ 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}, @@ -800,7 +800,7 @@ pub fn apply_workspace_edit( offset_encoding, ); let view = view_mut!(editor, view_id); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); doc.append_changes_to_history(view); }; diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index de24c4fba..bd91df5ae 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -4,10 +4,7 @@ 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)] @@ -480,7 +477,7 @@ fn set_line_ending( } }), ); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); doc.append_changes_to_history(view); Ok(()) @@ -925,7 +922,7 @@ fn replace_selections_with_clipboard_impl( (range.from(), range.to(), Some(contents.as_str().into())) }); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); doc.append_changes_to_history(view); Ok(()) } @@ -1596,7 +1593,7 @@ fn sort_impl( .map(|(s, fragment)| (s.from(), s.to(), Some(fragment))), ); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); doc.append_changes_to_history(view); Ok(()) @@ -1640,7 +1637,7 @@ fn reflow( (range.from(), range.to(), Some(reflowed_text)) }); - apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); doc.append_changes_to_history(view); view.ensure_cursor_in_view(doc, scrolloff); diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs index 824bafd8d..2eca709d1 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::{apply_transaction, editor::CompleteAction, ViewId}; +use helix_view::{editor::CompleteAction, ViewId}; use tui::buffer::Buffer as Surface; use std::borrow::Cow; @@ -183,7 +183,7 @@ fn completion_changes(transaction: &Transaction, trigger_offset: usize) -> Vec Vec Vec { let (_, doc) = current!(cxt.editor); diff --git a/helix-term/tests/test/helpers.rs b/helix-term/tests/test/helpers.rs index 2c5043d68..a0f3a32e4 100644 --- a/helix-term/tests/test/helpers.rs +++ b/helix-term/tests/test/helpers.rs @@ -130,7 +130,7 @@ pub async fn test_key_sequence_with_input_text>( }) .with_selection(test_case.in_selection.clone()); - helix_view::apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); test_key_sequence( &mut app, @@ -315,7 +315,7 @@ pub fn build(self) -> anyhow::Result { .with_selection(selection); // replace the initial text with the input text - helix_view::apply_transaction(&trans, doc, view); + doc.apply(&trans, view.id); } Ok(app) diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 856e5628a..6b33ea6aa 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -27,7 +27,7 @@ }; use crate::editor::RedrawHandle; -use crate::{apply_transaction, DocumentId, Editor, View, ViewId}; +use crate::{DocumentId, Editor, View, ViewId}; /// 8kB of buffer space for encoding and decoding `Rope`s. const BUF_SIZE: usize = 8192; @@ -650,7 +650,7 @@ pub fn reload( // 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); - apply_transaction(&transaction, self, view); + self.apply(&transaction, view.id); self.append_changes_to_history(view); self.reset_modified(); @@ -852,9 +852,6 @@ fn apply_impl(&mut self, transaction: &Transaction, view_id: ViewId) -> bool { } /// 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. @@ -911,7 +908,7 @@ pub fn savepoint(&mut self) { pub fn restore(&mut self, view: &mut View) { if let Some(revert) = self.savepoint.take() { - apply_transaction(&revert, self, view); + self.apply(&revert, view.id); } } diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 4a44a00cb..3ee0325d5 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1502,6 +1502,6 @@ fn inserted_a_new_blank_line(changes: &[Operation], pos: usize, line_end_pos: us let line_start_pos = text.line_to_char(range.cursor_line(text)); (line_start_pos, pos, None) }); - crate::apply_transaction(&transaction, doc, view); + doc.apply(&transaction, view.id); } } diff --git a/helix-view/src/lib.rs b/helix-view/src/lib.rs index 9cf36ae05..9a9804463 100644 --- a/helix-view/src/lib.rs +++ b/helix-view/src/lib.rs @@ -66,17 +66,6 @@ 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: &View, -) -> bool { - // TODO remove this helper function. Just call Document::apply everywhere directly. - doc.apply(transaction, view.id) -} - pub use document::Document; pub use editor::Editor; pub use theme::Theme; diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs index c09d502dc..23fb85c96 100644 --- a/helix-view/src/view.rs +++ b/helix-view/src/view.rs @@ -380,8 +380,6 @@ pub fn remove_document(&mut self, doc_id: &DocumentId) { // } /// 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: &mut Document) { self.jumps.apply(transaction, doc); self.doc_revisions