Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply transactions to views lazily #4912

Merged
merged 9 commits into from
Nov 29, 2022
34 changes: 9 additions & 25 deletions helix-core/src/history.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,32 +122,16 @@ impl History {
/// Returns the changes since the given revision composed into a transaction.
/// Returns None if there are no changes between the current and given revisions.
pub fn changes_since(&self, revision: usize) -> Option<Transaction> {
use std::cmp::Ordering::*;
let lca = self.lowest_common_ancestor(revision, self.current);
let up = self.path_up(revision, lca);
let down = self.path_up(self.current, lca);
let up_txns = up
.iter()
.rev()
.map(|&n| self.revisions[n].inversion.clone());
let down_txns = down.iter().map(|&n| self.revisions[n].transaction.clone());

match revision.cmp(&self.current) {
Equal => None,
Less => {
let mut child = self.revisions[revision].last_child?.get();
let mut transaction = self.revisions[child].transaction.clone();
while child != self.current {
child = self.revisions[child].last_child?.get();
transaction = transaction.compose(self.revisions[child].transaction.clone());
}
Some(transaction)
}
Greater => {
let mut inversion = self.revisions[revision].inversion.clone();
let mut parent = self.revisions[revision].parent;
while parent != self.current {
parent = self.revisions[parent].parent;
if parent == 0 {
return None;
}
inversion = inversion.compose(self.revisions[parent].inversion.clone());
}
Some(inversion)
}
}
up_txns.chain(down_txns).reduce(|acc, tx| tx.compose(acc))
}

/// Undo the last edit.
Expand Down
16 changes: 8 additions & 8 deletions helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2564,7 +2564,7 @@ async fn make_format_callback(
if let Ok(format) = format {
if doc.version() == doc_version {
apply_transaction(&format, doc, view);
doc.append_changes_to_history(view.id);
doc.append_changes_to_history(view);
doc.detect_indent_and_line_ending();
view.ensure_cursor_in_view(doc, scrolloff);
} else {
Expand Down Expand Up @@ -3321,7 +3321,7 @@ fn undo(cx: &mut Context) {
let count = cx.count();
let (view, doc) = current!(cx.editor);
for _ in 0..count {
if !doc.undo(view.id) {
if !doc.undo(view) {
cx.editor.set_status("Already at oldest change");
break;
}
Expand All @@ -3332,7 +3332,7 @@ fn redo(cx: &mut Context) {
let count = cx.count();
let (view, doc) = current!(cx.editor);
for _ in 0..count {
if !doc.redo(view.id) {
if !doc.redo(view) {
cx.editor.set_status("Already at newest change");
break;
}
Expand All @@ -3344,7 +3344,7 @@ fn earlier(cx: &mut Context) {
let (view, doc) = current!(cx.editor);
for _ in 0..count {
// rather than doing in batch we do this so get error halfway
if !doc.earlier(view.id, UndoKind::Steps(1)) {
if !doc.earlier(view, UndoKind::Steps(1)) {
cx.editor.set_status("Already at oldest change");
break;
}
Expand All @@ -3356,7 +3356,7 @@ fn later(cx: &mut Context) {
let (view, doc) = current!(cx.editor);
for _ in 0..count {
// rather than doing in batch we do this so get error halfway
if !doc.later(view.id, UndoKind::Steps(1)) {
if !doc.later(view, UndoKind::Steps(1)) {
cx.editor.set_status("Already at newest change");
break;
}
Expand All @@ -3365,7 +3365,7 @@ fn later(cx: &mut Context) {

fn commit_undo_checkpoint(cx: &mut Context) {
let (view, doc) = current!(cx.editor);
doc.append_changes_to_history(view.id);
doc.append_changes_to_history(view);
}

// Yank / Paste
Expand Down Expand Up @@ -3677,7 +3677,7 @@ fn replace_selections_with_clipboard_impl(
});

apply_transaction(&transaction, doc, view);
doc.append_changes_to_history(view.id);
doc.append_changes_to_history(view);
}
Err(e) => return Err(e.context("Couldn't get system clipboard contents")),
}
Expand Down Expand Up @@ -4884,7 +4884,7 @@ fn shell(cx: &mut compositor::Context, cmd: &str, behavior: &ShellBehavior) {
let transaction = Transaction::change(doc.text(), changes.into_iter())
.with_selection(Selection::new(ranges, selection.primary_index()));
apply_transaction(&transaction, doc, view);
doc.append_changes_to_history(view.id);
doc.append_changes_to_history(view);
}

// after replace cursor may be out of bounds, do this to
Expand Down
5 changes: 3 additions & 2 deletions helix-term/src/commands/lsp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -760,8 +760,9 @@ pub fn apply_workspace_edit(
text_edits,
offset_encoding,
);
apply_transaction(&transaction, doc, view_mut!(editor, view_id));
doc.append_changes_to_history(view_id);
let view = view_mut!(editor, view_id);
apply_transaction(&transaction, doc, view);
doc.append_changes_to_history(view);
};

if let Some(ref changes) = workspace_edit.changes {
Expand Down
12 changes: 6 additions & 6 deletions helix-term/src/commands/typed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ fn set_line_ending(
}),
);
apply_transaction(&transaction, doc, view);
doc.append_changes_to_history(view.id);
doc.append_changes_to_history(view);

Ok(())
}
Expand All @@ -481,7 +481,7 @@ fn earlier(
let uk = args.join(" ").parse::<UndoKind>().map_err(|s| anyhow!(s))?;

let (view, doc) = current!(cx.editor);
let success = doc.earlier(view.id, uk);
let success = doc.earlier(view, uk);
if !success {
cx.editor.set_status("Already at oldest change");
}
Expand All @@ -500,7 +500,7 @@ fn later(

let uk = args.join(" ").parse::<UndoKind>().map_err(|s| anyhow!(s))?;
let (view, doc) = current!(cx.editor);
let success = doc.later(view.id, uk);
let success = doc.later(view, uk);
if !success {
cx.editor.set_status("Already at newest change");
}
Expand Down Expand Up @@ -909,7 +909,7 @@ fn replace_selections_with_clipboard_impl(
});

apply_transaction(&transaction, doc, view);
doc.append_changes_to_history(view.id);
doc.append_changes_to_history(view);
Ok(())
}
Err(e) => Err(e.context("Couldn't get system clipboard contents")),
Expand Down Expand Up @@ -1573,7 +1573,7 @@ fn sort_impl(
);

apply_transaction(&transaction, doc, view);
doc.append_changes_to_history(view.id);
doc.append_changes_to_history(view);

Ok(())
}
Expand Down Expand Up @@ -1617,7 +1617,7 @@ fn reflow(
});

apply_transaction(&transaction, doc, view);
doc.append_changes_to_history(view.id);
doc.append_changes_to_history(view);
view.ensure_cursor_in_view(doc, scrolloff);

Ok(())
Expand Down
30 changes: 5 additions & 25 deletions helix-term/src/ui/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1319,7 +1319,7 @@ impl Component for EditorView {
// Store a history state if not in insert mode. Otherwise wait till we exit insert
// to include any edits to the paste in the history state.
if mode != Mode::Insert {
doc.append_changes_to_history(view.id);
doc.append_changes_to_history(view);
}

EventResult::Consumed(None)
Expand All @@ -1337,9 +1337,7 @@ impl Component for EditorView {
cx.editor.status_msg = None;

let mode = cx.editor.mode();
let (view, doc) = current!(cx.editor);
let original_doc_id = doc.id();
let original_doc_revision = doc.get_current_revision();
let (view, _) = current!(cx.editor);
let focus = view.id;

if let Some(on_next_key) = self.on_next_key.take() {
Expand Down Expand Up @@ -1415,31 +1413,13 @@ impl Component for EditorView {
let view = view_mut!(cx.editor, focus);
let doc = doc_mut!(cx.editor, &view.doc);

view.ensure_cursor_in_view(doc, config.scrolloff);

// Store a history state if not in insert mode. This also takes care of
// committing changes when leaving insert mode.
if mode != Mode::Insert {
doc.append_changes_to_history(view.id);
doc.append_changes_to_history(view);
}

// If the current document has been changed, apply the changes to all views.
// This ensures that selections in jumplists follow changes.
if doc.id() == original_doc_id
&& doc.get_current_revision() != original_doc_revision
{
if let Some(transaction) =
doc.history.get_mut().changes_since(original_doc_revision)
{
let doc = doc!(cx.editor, &original_doc_id);
for (view, _focused) in cx.editor.tree.views_mut() {
view.apply(&transaction, doc);
}
}
}

let view = view_mut!(cx.editor, focus);
let doc = doc_mut!(cx.editor, &view.doc);

view.ensure_cursor_in_view(doc, config.scrolloff);
}

EventResult::Consumed(callback)
Expand Down
10 changes: 10 additions & 0 deletions helix-term/tests/test/splits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,5 +151,15 @@ async fn test_changes_in_splits_apply_to_all_views() -> anyhow::Result<()> {
// was not updated and after the `kd` step, pointed outside of the document.
test(("#[|]#", "<C-w>v[<space><C-s><C-w>wkd<C-w>qd", "#[|]#")).await?;

// Transactions are applied to the views for windows lazily when they are focused.
// This case panics if the transactions and inversions are not applied in the
// correct order as we switch between windows.
test((
"#[|]#",
"[<space>[<space>[<space><C-w>vuuu<C-w>wUUU<C-w>quuu",
"#[|]#",
))
.await?;

Ok(())
}
37 changes: 22 additions & 15 deletions helix-view/src/document.rs
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ impl Document {
// of the encoding.
let transaction = helix_core::diff::compare_ropes(self.text(), &rope);
apply_transaction(&transaction, self, view);
self.append_changes_to_history(view.id);
self.append_changes_to_history(view);
self.reset_modified();

self.detect_indent_and_line_ending();
Expand Down Expand Up @@ -857,11 +857,11 @@ impl Document {
success
}

fn undo_redo_impl(&mut self, view_id: ViewId, undo: bool) -> bool {
fn undo_redo_impl(&mut self, view: &mut View, undo: bool) -> bool {
let mut history = self.history.take();
let txn = if undo { history.undo() } else { history.redo() };
let success = if let Some(txn) = txn {
self.apply_impl(txn, view_id)
self.apply_impl(txn, view.id)
} else {
false
};
Expand All @@ -870,18 +870,20 @@ impl Document {
if success {
// reset changeset to fix len
self.changes = ChangeSet::new(self.text());
// Sync with changes with the jumplist selections.
view.sync_changes(self);
}
success
}

/// Undo the last modification to the [`Document`]. Returns whether the undo was successful.
pub fn undo(&mut self, view_id: ViewId) -> bool {
self.undo_redo_impl(view_id, true)
pub fn undo(&mut self, view: &mut View) -> bool {
self.undo_redo_impl(view, true)
}

/// Redo the last modification to the [`Document`]. Returns whether the redo was successful.
pub fn redo(&mut self, view_id: ViewId) -> bool {
self.undo_redo_impl(view_id, false)
pub fn redo(&mut self, view: &mut View) -> bool {
self.undo_redo_impl(view, false)
}

pub fn savepoint(&mut self) {
Expand All @@ -894,37 +896,39 @@ impl Document {
}
}

fn earlier_later_impl(&mut self, view_id: ViewId, uk: UndoKind, earlier: bool) -> bool {
fn earlier_later_impl(&mut self, view: &mut View, uk: UndoKind, earlier: bool) -> bool {
let txns = if earlier {
self.history.get_mut().earlier(uk)
} else {
self.history.get_mut().later(uk)
};
let mut success = false;
for txn in txns {
if self.apply_impl(&txn, view_id) {
if self.apply_impl(&txn, view.id) {
success = true;
}
}
if success {
// reset changeset to fix len
self.changes = ChangeSet::new(self.text());
// Sync with changes with the jumplist selections.
view.sync_changes(self);
}
success
}

/// Undo modifications to the [`Document`] according to `uk`.
pub fn earlier(&mut self, view_id: ViewId, uk: UndoKind) -> bool {
self.earlier_later_impl(view_id, uk, true)
pub fn earlier(&mut self, view: &mut View, uk: UndoKind) -> bool {
self.earlier_later_impl(view, uk, true)
}

/// Redo modifications to the [`Document`] according to `uk`.
pub fn later(&mut self, view_id: ViewId, uk: UndoKind) -> bool {
self.earlier_later_impl(view_id, uk, false)
pub fn later(&mut self, view: &mut View, uk: UndoKind) -> bool {
self.earlier_later_impl(view, uk, false)
}

/// Commit pending changes to history
pub fn append_changes_to_history(&mut self, view_id: ViewId) {
pub fn append_changes_to_history(&mut self, view: &mut View) {
if self.changes.is_empty() {
return;
}
Expand All @@ -934,14 +938,17 @@ impl Document {
// Instead of doing this messy merge we could always commit, and based on transaction
// annotations either add a new layer or compose into the previous one.
let transaction =
Transaction::from(changes).with_selection(self.selection(view_id).clone());
Transaction::from(changes).with_selection(self.selection(view.id).clone());

// HAXX: we need to reconstruct the state as it was before the changes..
let old_state = self.old_state.take().expect("no old_state available");

let mut history = self.history.take();
history.commit_revision(&transaction, &old_state);
self.history.set(history);

// Update jumplist entries in the view.
view.apply(&transaction, self);
}

pub fn id(&self) -> DocumentId {
Expand Down
10 changes: 9 additions & 1 deletion helix-view/src/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,8 @@ impl Editor {
fn _refresh(&mut self) {
let config = self.config();
for (view, _) in self.tree.views_mut() {
let doc = &self.documents[&view.doc];
let doc = doc_mut!(self, &view.doc);
view.sync_changes(doc);
view.ensure_cursor_in_view(doc, config.scrolloff)
}
}
Expand All @@ -971,6 +972,7 @@ impl Editor {

let doc = doc_mut!(self, &doc_id);
doc.ensure_view_init(view.id);
view.sync_changes(doc);

align_view(doc, view, Align::Center);
}
Expand Down Expand Up @@ -1240,6 +1242,12 @@ impl Editor {
if prev_id != view_id {
self.mode = Mode::Normal;
self.ensure_cursor_in_view(view_id);

// Update jumplist selections with new document changes.
for (view, _focused) in self.tree.views_mut() {
let doc = doc_mut!(self, &view.doc);
view.sync_changes(doc);
}
}
}

Expand Down
Loading