Skip to content

Commit

Permalink
Don't apply transactions to Views in undo/redo
Browse files Browse the repository at this point in the history
View::apply should only be called by EditorView after
42e37a5. This change removes the
duplicate calls within undo/redo which could cause a panic.
  • Loading branch information
the-mikedavis authored and archseer committed Nov 24, 2022
1 parent 94eb3de commit fd00f3a
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 18 deletions.
8 changes: 4 additions & 4 deletions helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3302,7 +3302,7 @@ fn undo(cx: &mut Context) {
let count = cx.count();
let (view, doc) = current!(cx.editor);
for _ in 0..count {
if !doc.undo(view) {
if !doc.undo(view.id) {
cx.editor.set_status("Already at oldest change");
break;
}
Expand All @@ -3313,7 +3313,7 @@ fn redo(cx: &mut Context) {
let count = cx.count();
let (view, doc) = current!(cx.editor);
for _ in 0..count {
if !doc.redo(view) {
if !doc.redo(view.id) {
cx.editor.set_status("Already at newest change");
break;
}
Expand All @@ -3325,7 +3325,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, UndoKind::Steps(1)) {
if !doc.earlier(view.id, UndoKind::Steps(1)) {
cx.editor.set_status("Already at oldest change");
break;
}
Expand All @@ -3337,7 +3337,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, UndoKind::Steps(1)) {
if !doc.later(view.id, UndoKind::Steps(1)) {
cx.editor.set_status("Already at newest change");
break;
}
Expand Down
4 changes: 2 additions & 2 deletions helix-term/src/commands/typed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, uk);
let success = doc.earlier(view.id, 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, uk);
let success = doc.later(view.id, uk);
if !success {
cx.editor.set_status("Already at newest change");
}
Expand Down
24 changes: 12 additions & 12 deletions helix-view/src/document.rs
Original file line number Diff line number Diff line change
Expand Up @@ -857,11 +857,11 @@ impl Document {
success
}

fn undo_redo_impl(&mut self, view: &mut View, undo: bool) -> bool {
fn undo_redo_impl(&mut self, view_id: ViewId, 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) && view.apply(txn, self)
self.apply_impl(txn, view_id)
} else {
false
};
Expand All @@ -875,13 +875,13 @@ impl Document {
}

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

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

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

fn earlier_later_impl(&mut self, view: &mut View, uk: UndoKind, earlier: bool) -> bool {
fn earlier_later_impl(&mut self, view_id: ViewId, 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) && view.apply(&txn, self) {
if self.apply_impl(&txn, view_id) {
success = true;
}
}
Expand All @@ -914,13 +914,13 @@ impl Document {
}

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

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

/// Commit pending changes to history
Expand Down

0 comments on commit fd00f3a

Please sign in to comment.