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.
pull/4192/head
Michael Davis 2 years ago committed by Blaž Hrastnik
parent 0aedef0333
commit c388e16e09

@ -27,6 +27,7 @@ use helix_core::{
SmallVec, Tendril, Transaction, SmallVec, Tendril, Transaction,
}; };
use helix_view::{ use helix_view::{
apply_transaction,
clipboard::ClipboardType, clipboard::ClipboardType,
document::{FormatterError, Mode, SCRATCH_BUFFER_NAME}, document::{FormatterError, Mode, SCRATCH_BUFFER_NAME},
editor::{Action, Motion}, editor::{Action, Motion},
@ -859,8 +860,7 @@ fn align_selections(cx: &mut Context) {
changes.sort_unstable_by_key(|(from, _, _)| *from); changes.sort_unstable_by_key(|(from, _, _)| *from);
let transaction = Transaction::change(doc.text(), changes.into_iter()); let transaction = Transaction::change(doc.text(), changes.into_iter());
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
} }
fn goto_window(cx: &mut Context, align: Align) { fn goto_window(cx: &mut Context, align: Align) {
@ -1290,8 +1290,7 @@ fn replace(cx: &mut Context) {
} }
}); });
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
} }
}) })
} }
@ -1308,8 +1307,7 @@ where
(range.from(), range.to(), Some(text)) (range.from(), range.to(), Some(text))
}); });
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
} }
fn switch_case(cx: &mut Context) { 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| { let transaction = Transaction::change_by_selection(doc.text(), selection, |range| {
(range.from(), range.to(), None) (range.from(), range.to(), None)
}); });
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
match op { match op {
Operation::Delete => { Operation::Delete => {
@ -2131,14 +2128,10 @@ fn delete_selection_impl(cx: &mut Context, op: Operation) {
#[inline] #[inline]
fn delete_selection_insert_mode(doc: &mut Document, view: &mut View, selection: &Selection) { 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| { let transaction = Transaction::change_by_selection(doc.text(), selection, |range| {
(range.from(), range.to(), None) (range.from(), range.to(), None)
}); });
doc.apply(&transaction, view_id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
} }
fn delete_selection(cx: &mut Context) { fn delete_selection(cx: &mut Context) {
@ -2234,8 +2227,7 @@ fn append_mode(cx: &mut Context) {
doc.text(), doc.text(),
[(end, end, Some(doc.line_ending.as_str().into()))].into_iter(), [(end, end, Some(doc.line_ending.as_str().into()))].into_iter(),
); );
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
} }
let selection = doc.selection(view.id).clone().transform(|range| { 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 doc = doc_mut!(editor, &doc_id);
let view = view_mut!(editor); let view = view_mut!(editor);
if doc.version() == doc_version { if doc.version() == doc_version {
doc.apply(&format, view.id); apply_transaction(&format, doc, view);
view.apply(&format, doc);
doc.append_changes_to_history(view.id); doc.append_changes_to_history(view.id);
doc.detect_indent_and_line_ending(); doc.detect_indent_and_line_ending();
view.ensure_cursor_in_view(doc, scrolloff); 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())); transaction = transaction.with_selection(Selection::new(ranges, selection.primary_index()));
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
} }
// o inserts a new line after each line with a selection // 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)); let line_start_pos = text.line_to_char(range.cursor_line(text));
(line_start_pos, pos, None) (line_start_pos, pos, None)
}); });
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
} }
} }
@ -3007,8 +2996,7 @@ pub mod insert {
let (view, doc) = current!(cx.editor); let (view, doc) = current!(cx.editor);
if let Some(t) = transaction { if let Some(t) = transaction {
doc.apply(&t, view.id); apply_transaction(&t, doc, view);
view.apply(&t, doc);
} }
// TODO: need a post insert hook too for certain triggers (autocomplete, signature help, etc) // 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(..)), &doc.selection(view.id).clone().cursors(doc.text().slice(..)),
indent, indent,
); );
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
} }
pub fn insert_newline(cx: &mut Context) { pub fn insert_newline(cx: &mut Context) {
@ -3118,8 +3105,7 @@ pub mod insert {
transaction = transaction.with_selection(Selection::new(ranges, selection.primary_index())); transaction = transaction.with_selection(Selection::new(ranges, selection.primary_index()));
let (view, doc) = current!(cx.editor); let (view, doc) = current!(cx.editor);
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
} }
pub fn delete_char_backward(cx: &mut Context) { pub fn delete_char_backward(cx: &mut Context) {
@ -3213,8 +3199,7 @@ pub mod insert {
} }
}); });
let (view, doc) = current!(cx.editor); let (view, doc) = current!(cx.editor);
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
lsp::signature_help_impl(cx, SignatureHelpInvoked::Automatic); lsp::signature_help_impl(cx, SignatureHelpInvoked::Automatic);
} }
@ -3232,8 +3217,7 @@ pub mod insert {
None, None,
) )
}); });
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
lsp::signature_help_impl(cx, SignatureHelpInvoked::Automatic); 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()) (pos, pos, values.next())
}); });
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
} }
pub(crate) fn paste_bracketed_value(cx: &mut Context, contents: String) { 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); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
} }
} }
} }
@ -3587,8 +3569,7 @@ fn replace_selections_with_clipboard_impl(
) )
}); });
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
doc.append_changes_to_history(view.id); doc.append_changes_to_history(view.id);
Ok(()) Ok(())
} }
@ -3658,8 +3639,7 @@ fn indent(cx: &mut Context) {
Some((pos, pos, Some(indent.clone()))) Some((pos, pos, Some(indent.clone())))
}), }),
); );
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
} }
fn unindent(cx: &mut Context) { fn unindent(cx: &mut Context) {
@ -3698,8 +3678,7 @@ fn unindent(cx: &mut Context) {
let transaction = Transaction::change(doc.text(), changes.into_iter()); let transaction = Transaction::change(doc.text(), changes.into_iter());
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
} }
fn format_selections(cx: &mut Context) { fn format_selections(cx: &mut Context) {
@ -3746,8 +3725,7 @@ fn format_selections(cx: &mut Context) {
// language_server.offset_encoding(), // language_server.offset_encoding(),
// ); // );
// doc.apply(&transaction, view.id); // apply_transaction(&transaction, doc, view);
// view.apply(&transaction, doc);
} }
} }
@ -3802,8 +3780,7 @@ fn join_selections_inner(cx: &mut Context, select_space: bool) {
Transaction::change(doc.text(), changes.into_iter()) Transaction::change(doc.text(), changes.into_iter())
}; };
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
} }
fn keep_or_remove_selections_impl(cx: &mut Context, remove: bool) { 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()); .map(|tc| tc.as_ref());
let transaction = comment::toggle_line_comments(doc.text(), doc.selection(view.id), token); let transaction = comment::toggle_line_comments(doc.text(), doc.selection(view.id), token);
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
exit_select_mode(cx); 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))), .map(|(range, fragment)| (range.from(), range.to(), Some(fragment))),
); );
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
} }
fn rotate_selection_contents_forward(cx: &mut Context) { 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()); let transaction = Transaction::change(doc.text(), changes.into_iter());
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
}) })
} }
@ -4550,8 +4524,7 @@ fn surround_replace(cx: &mut Context) {
(pos, pos + 1, Some(t)) (pos, pos + 1, Some(t))
}), }),
); );
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
}); });
}) })
} }
@ -4578,8 +4551,7 @@ fn surround_delete(cx: &mut Context) {
let transaction = let transaction =
Transaction::change(doc.text(), change_pos.into_iter().map(|p| (p, p + 1, None))); Transaction::change(doc.text(), change_pos.into_iter().map(|p| (p, p + 1, None)));
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
}) })
} }
@ -4754,8 +4726,7 @@ fn shell(cx: &mut compositor::Context, cmd: &str, behavior: &ShellBehavior) {
if behavior != &ShellBehavior::Ignore { if behavior != &ShellBehavior::Ignore {
let transaction = Transaction::change(doc.text(), changes.into_iter()); let transaction = Transaction::change(doc.text(), changes.into_iter());
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
doc.append_changes_to_history(view.id); 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); let transaction = Transaction::change(text, changes);
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
} }
/// Increment object under cursor by count. /// 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::change(doc.text(), changes);
let transaction = transaction.with_selection(selection.clone()); let transaction = transaction.with_selection(selection.clone());
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
} }
} }

@ -9,7 +9,7 @@ use tui::text::{Span, Spans};
use super::{align_view, push_jump, Align, Context, Editor, Open}; use super::{align_view, push_jump, Align, Context, Editor, Open};
use helix_core::{path, Selection}; use helix_core::{path, Selection};
use helix_view::{editor::Action, theme::Style}; use helix_view::{apply_transaction, editor::Action, theme::Style};
use crate::{ use crate::{
compositor::{self, Compositor}, compositor::{self, Compositor},
@ -617,8 +617,7 @@ pub fn apply_workspace_edit(
text_edits, text_edits,
offset_encoding, offset_encoding,
); );
doc.apply(&transaction, view_id); apply_transaction(&transaction, doc, view_mut!(editor, view_id));
view_mut!(editor, view_id).apply(&transaction, doc);
doc.append_changes_to_history(view_id); doc.append_changes_to_history(view_id);
}; };

@ -2,7 +2,10 @@ use std::ops::Deref;
use super::*; use super::*;
use helix_view::editor::{Action, CloseError, ConfigEvent}; use helix_view::{
apply_transaction,
editor::{Action, CloseError, ConfigEvent},
};
use ui::completers::{self, Completer}; use ui::completers::{self, Completer};
#[derive(Clone)] #[derive(Clone)]
@ -462,8 +465,7 @@ fn set_line_ending(
} }
}), }),
); );
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
doc.append_changes_to_history(view.id); doc.append_changes_to_history(view.id);
Ok(()) Ok(())
@ -884,8 +886,7 @@ fn replace_selections_with_clipboard_impl(
(range.from(), range.to(), Some(contents.as_str().into())) (range.from(), range.to(), Some(contents.as_str().into()))
}); });
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
doc.append_changes_to_history(view.id); doc.append_changes_to_history(view.id);
Ok(()) Ok(())
} }
@ -1400,8 +1401,7 @@ fn sort_impl(
.map(|(s, fragment)| (s.from(), s.to(), Some(fragment))), .map(|(s, fragment)| (s.from(), s.to(), Some(fragment))),
); );
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
doc.append_changes_to_history(view.id); doc.append_changes_to_history(view.id);
Ok(()) Ok(())
@ -1445,8 +1445,7 @@ fn reflow(
(range.from(), range.to(), Some(reflowed_text)) (range.from(), range.to(), Some(reflowed_text))
}); });
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
doc.append_changes_to_history(view.id); doc.append_changes_to_history(view.id);
view.ensure_cursor_in_view(doc, scrolloff); view.ensure_cursor_in_view(doc, scrolloff);

@ -1,5 +1,5 @@
use crate::compositor::{Component, Context, Event, EventResult}; 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::buffer::Buffer as Surface;
use tui::text::Spans; use tui::text::Spans;
@ -164,8 +164,7 @@ impl Completion {
// initialize a savepoint // initialize a savepoint
doc.savepoint(); doc.savepoint();
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
editor.last_completion = Some(CompleteAction { editor.last_completion = Some(CompleteAction {
trigger_offset, trigger_offset,
@ -184,8 +183,7 @@ impl Completion {
trigger_offset, trigger_offset,
); );
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
editor.last_completion = Some(CompleteAction { editor.last_completion = Some(CompleteAction {
trigger_offset, trigger_offset,
@ -215,8 +213,7 @@ impl Completion {
additional_edits.clone(), additional_edits.clone(),
offset_encoding, // TODO: should probably transcode in Client offset_encoding, // TODO: should probably transcode in Client
); );
doc.apply(&transaction, view.id); apply_transaction(&transaction, doc, view);
view.apply(&transaction, doc);
} }
} }
} }

@ -16,6 +16,7 @@ use helix_core::{
visual_coords_at_pos, LineEnding, Position, Range, Selection, Transaction, visual_coords_at_pos, LineEnding, Position, Range, Selection, Transaction,
}; };
use helix_view::{ use helix_view::{
apply_transaction,
document::{Mode, SCRATCH_BUFFER_NAME}, document::{Mode, SCRATCH_BUFFER_NAME},
editor::{CompleteAction, CursorShapeConfig}, editor::{CompleteAction, CursorShapeConfig},
graphics::{Color, CursorKind, Modifier, Rect, Style}, graphics::{Color, CursorKind, Modifier, Rect, Style},
@ -1002,8 +1003,7 @@ impl EditorView {
(shift_position(start), shift_position(end), t) (shift_position(start), shift_position(end), t)
}), }),
); );
doc.apply(&tx, view.id); apply_transaction(&tx, doc, view);
view.apply(&tx, doc);
} }
InsertEvent::TriggerCompletion => { InsertEvent::TriggerCompletion => {
let (_, doc) = current!(cxt.editor); let (_, doc) = current!(cxt.editor);

@ -24,7 +24,7 @@ use helix_core::{
DEFAULT_LINE_ENDING, 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. /// 8kB of buffer space for encoding and decoding `Rope`s.
const BUF_SIZE: usize = 8192; const BUF_SIZE: usize = 8192;
@ -617,8 +617,7 @@ impl Document {
// This is not considered a modification of the contents of the file regardless // This is not considered a modification of the contents of the file regardless
// of the encoding. // of the encoding.
let transaction = helix_core::diff::compare_ropes(self.text(), &rope); let transaction = helix_core::diff::compare_ropes(self.text(), &rope);
self.apply(&transaction, view.id); apply_transaction(&transaction, self, view);
view.apply(&transaction, self);
self.append_changes_to_history(view.id); self.append_changes_to_history(view.id);
self.reset_modified(); self.reset_modified();
@ -811,6 +810,9 @@ impl Document {
} }
/// Apply a [`Transaction`] to the [`Document`] to change its text. /// 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 { 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 // store the state just before any changes are made. This allows us to undo to the
// state just before a transaction was applied. // state just before a transaction was applied.
@ -865,8 +867,7 @@ impl Document {
pub fn restore(&mut self, view: &mut View) { pub fn restore(&mut self, view: &mut View) {
if let Some(revert) = self.savepoint.take() { if let Some(revert) = self.savepoint.take() {
self.apply(&revert, view.id); apply_transaction(&revert, self, view);
view.apply(&revert, self);
} }
} }

@ -64,6 +64,19 @@ pub fn align_view(doc: &Document, view: &mut View, align: Align) {
view.offset.row = line.saturating_sub(relative); 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 document::Document;
pub use editor::Editor; pub use editor::Editor;
pub use theme::Theme; pub use theme::Theme;

Loading…
Cancel
Save