Skip to content

Commit

Permalink
Apply transactions to all views
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@the-mikedavis
the-mikedavis committed Nov 13, 2022
1 parent fd585c1 commit aca5c96
Show file tree
Hide file tree
Showing 10 changed files with 315 additions and 207 deletions.
211 changes: 114 additions & 97 deletions helix-term/src/commands.rs
View file

Large diffs are not rendered by default.

7 changes: 4 additions & 3 deletions helix-term/src/commands/lsp.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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();
Expand All @@ -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);
};

Expand Down
67 changes: 31 additions & 36 deletions helix-term/src/commands/typed.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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(
Expand All @@ -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(())
Expand All @@ -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");
}

Expand All @@ -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");
}

Expand Down Expand Up @@ -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) => {
Expand All @@ -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(())
}
Expand Down Expand Up @@ -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.
Expand All @@ -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(())
Expand All @@ -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
Expand Down Expand Up @@ -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")?;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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(())
Expand All @@ -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;

Expand Down Expand Up @@ -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);

Expand All @@ -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();
Expand Down
23 changes: 12 additions & 11 deletions helix-term/src/ui/completion.rs
View file
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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
Expand All @@ -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);
}
}
}
Expand Down
9 changes: 4 additions & 5 deletions helix-term/src/ui/editor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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);

Expand All @@ -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);
Expand Down
Loading

0 comments on commit aca5c96

Please sign in to comment.