From f5f46b1fed242f0b5a206753f7f977299fb2ff65 Mon Sep 17 00:00:00 2001 From: Ivan Tham Date: Wed, 2 Jun 2021 23:47:50 +0800 Subject: [PATCH] Separate document history into Cell As history is used separately from the rest of the edits, separating it can avoid needless borrowing and cloning. But one need to be aware later. --- helix-core/src/history.rs | 10 ++++------ helix-core/src/transaction.rs | 2 +- helix-view/src/document.rs | 30 ++++++++++++++++++++++-------- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/helix-core/src/history.rs b/helix-core/src/history.rs index 5a9ec8de..4348ee32 100644 --- a/helix-core/src/history.rs +++ b/helix-core/src/history.rs @@ -65,9 +65,7 @@ impl History { self.cursor == 0 } - // TODO: I'd like to pass Transaction by reference but it fights with the borrowck - - pub fn undo(&mut self) -> Option { + pub fn undo(&mut self) -> Option<&Transaction> { if self.at_root() { // We're at the root of undo, nothing to do. return None; @@ -77,17 +75,17 @@ impl History { self.cursor = current_revision.parent; - Some(current_revision.revert.clone()) + Some(¤t_revision.revert) } - pub fn redo(&mut self) -> Option { + pub fn redo(&mut self) -> Option<&Transaction> { let current_revision = &self.revisions[self.cursor]; // for now, simply pick the latest child (linear undo / redo) if let Some((index, transaction)) = current_revision.children.last() { self.cursor = *index; - return Some(transaction.clone()); + return Some(&transaction); } None } diff --git a/helix-core/src/transaction.rs b/helix-core/src/transaction.rs index 77cb358f..e61063f0 100644 --- a/helix-core/src/transaction.rs +++ b/helix-core/src/transaction.rs @@ -415,7 +415,7 @@ impl ChangeSet { /// Transaction represents a single undoable unit of changes. Several changes can be grouped into /// a single transaction. -#[derive(Debug, Clone)] +#[derive(Debug, Default, Clone)] pub struct Transaction { changes: ChangeSet, selection: Option, diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 51d8a795..3a3b9390 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -1,4 +1,5 @@ use anyhow::{Context, Error}; +use std::cell::Cell; use std::future::Future; use std::path::{Component, Path, PathBuf}; use std::sync::Arc; @@ -40,7 +41,10 @@ pub struct Document { /// State at last commit. Used for calculating reverts. old_state: Option, /// Undo tree. - history: History, + // It can be used as a cell where we will take it out to get some parts of the history and put + // it back as it separated from the edits. We could split out the parts manually but that will + // be more troublesome. + history: Cell, last_saved_revision: usize, version: i32, // should be usize? @@ -121,7 +125,7 @@ impl Document { old_state, diagnostics: Vec::new(), version: 0, - history: History::default(), + history: Cell::new(History::default()), last_saved_revision: 0, language_server: None, } @@ -190,7 +194,9 @@ impl Document { let language_server = self.language_server.clone(); // reset the modified flag - self.last_saved_revision = self.history.current_revision(); + let history = self.history.take(); + self.last_saved_revision = history.current_revision(); + self.history.set(history); async move { use tokio::{fs::File, io::AsyncWriteExt}; @@ -335,7 +341,8 @@ impl Document { } pub fn undo(&mut self, view_id: ViewId) -> bool { - if let Some(transaction) = self.history.undo() { + let mut history = self.history.take(); + if let Some(transaction) = history.undo() { let success = self._apply(&transaction, view_id); // reset changeset to fix len @@ -343,11 +350,13 @@ impl Document { return success; } + self.history.set(history); false } pub fn redo(&mut self, view_id: ViewId) -> bool { - if let Some(transaction) = self.history.redo() { + let mut history = self.history.take(); + if let Some(transaction) = history.redo() { let success = self._apply(&transaction, view_id); // reset changeset to fix len @@ -355,6 +364,7 @@ impl Document { return success; } + self.history.set(history); false } @@ -373,7 +383,9 @@ impl Document { // 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"); - self.history.commit_revision(&transaction, &old_state); + let mut history = self.history.take(); + history.commit_revision(&transaction, &old_state); + self.history.set(history); } #[inline] @@ -383,9 +395,11 @@ impl Document { #[inline] pub fn is_modified(&self) -> bool { + let history = self.history.take(); + let current_revision = history.current_revision(); + self.history.set(history); self.path.is_some() - && (self.history.current_revision() != self.last_saved_revision - || !self.changes.is_empty()) + && (current_revision != self.last_saved_revision || !self.changes.is_empty()) } #[inline]