dino created
* Introduce `project_panel::Redo` action
* Add the necessary handler for the `project_panel::Redo` action
* For the time being, the redo implementation doesn't actually redo
anything at all as that will be added in a future commit, this one
was more focused on actually shipping the cursor-based tracking of
undo history in order to support redoing operations
* Introduce `UndoManager::cursor` in order to allow tracking where the
current position is in the undo history, which also makes it easier to
re-use the undo history for redoing operations, without the need to
introduce another data structure to hold the redo history
* Update default platform keymaps to include a binding for
`project_panel::Redo`assets/keymaps/default-linux.json | 2
assets/keymaps/default-macos.json | 1
assets/keymaps/default-windows.json | 1
crates/project_panel/src/project_panel.rs | 15 +
crates/project_panel/src/undo.rs | 212 +++++++++++++++++++++---
5 files changed, 205 insertions(+), 26 deletions(-)
@@ -892,6 +892,8 @@
"alt-ctrl-shift-c": "workspace::CopyRelativePath",
"undo": "project_panel::Undo",
"ctrl-z": "project_panel::Undo",
+ "redo": "project_panel::Redo",
+ "ctrl-shift-z": "project_panel::Redo",
"enter": "project_panel::Rename",
"f2": "project_panel::Rename",
"backspace": ["project_panel::Trash", { "skip_prompt": false }],
@@ -952,6 +952,7 @@
"cmd-alt-c": "workspace::CopyPath",
"alt-cmd-shift-c": "workspace::CopyRelativePath",
"cmd-z": "project_panel::Undo",
+ "cmd-shift-z": "project_panel::Redo",
"enter": "project_panel::Rename",
"f2": "project_panel::Rename",
"backspace": ["project_panel::Trash", { "skip_prompt": false }],
@@ -888,6 +888,7 @@
"shift-alt-c": "project_panel::CopyPath",
"ctrl-k ctrl-shift-c": "workspace::CopyRelativePath",
"ctrl-z": "project_panel::Undo",
+ "ctrl-shift-z": "project_panel::Redo",
"enter": "project_panel::Rename",
"f2": "project_panel::Rename",
"backspace": ["project_panel::Trash", { "skip_prompt": false }],
@@ -401,6 +401,10 @@ actions!(
CompareMarkedFiles,
/// Undoes the last file operation.
Undo,
+ // TODO!: Improve documentation, this is not really what's happening is
+ // it?
+ /// Redoes the last undone file operation.
+ Redo,
]
);
@@ -1243,6 +1247,11 @@ impl ProjectPanel {
"Undo",
Box::new(Undo),
)
+ .action_disabled_when(
+ !self.undo_manager.can_redo(),
+ "Redo",
+ Box::new(Redo),
+ )
})
.when(is_remote, |menu| {
menu.separator()
@@ -2208,6 +2217,11 @@ impl ProjectPanel {
cx.notify();
}
+ pub fn redo(&mut self, _: &Redo, _window: &mut Window, cx: &mut Context<Self>) {
+ self.undo_manager.redo(cx);
+ cx.notify();
+ }
+
fn rename_impl(
&mut self,
selection: Option<Range<usize>>,
@@ -6684,6 +6698,7 @@ impl Render for ProjectPanel {
.on_action(cx.listener(Self::compare_marked_files))
.when(cx.has_flag::<ProjectPanelUndoRedoFeatureFlag>(), |el| {
el.on_action(cx.listener(Self::undo))
+ .on_action(cx.listener(Self::redo))
})
.when(!project.is_read_only(cx), |el| {
el.on_action(cx.listener(Self::new_file))
@@ -10,7 +10,7 @@ use workspace::{
const MAX_UNDO_OPERATIONS: usize = 10_000;
-#[derive(Clone)]
+#[derive(Clone, Debug, PartialEq)]
pub enum ProjectPanelOperation {
Batch(Vec<ProjectPanelOperation>),
Create {
@@ -24,8 +24,13 @@ pub enum ProjectPanelOperation {
pub struct UndoManager {
workspace: WeakEntity<Workspace>,
- stack: VecDeque<ProjectPanelOperation>,
- /// Maximum number of operations to keep on the undo stack.
+ history: VecDeque<ProjectPanelOperation>,
+ /// Keeps track of the cursor position in the undo stack so we can easily
+ /// undo by picking the current operation in the stack and decreasing the
+ /// cursor, as well as redoing, by picking the next operation in the stack
+ /// and increasing the cursor.
+ cursor: usize,
+ /// Maximum number of operations to keep on the undo history.
limit: usize,
}
@@ -38,17 +43,32 @@ impl UndoManager {
Self {
workspace,
limit,
- stack: VecDeque::new(),
+ cursor: 0,
+ history: VecDeque::new(),
}
}
pub fn can_undo(&self) -> bool {
- !self.stack.is_empty()
+ self.cursor > 0
+ }
+
+ pub fn can_redo(&self) -> bool {
+ self.cursor < self.history.len()
}
pub fn undo(&mut self, cx: &mut App) {
- if let Some(operation) = self.stack.pop_back() {
- let task = self.revert_operation(operation, cx);
+ if self.cursor == 0 {
+ return;
+ }
+
+ // We don't currently care whether the undo operation failed or
+ // succeeded, so the cursor can always be updated, as we just assume
+ // we'll be attempting to undo the next operation, even if undoing
+ // the previous one failed.
+ self.cursor -= 1;
+
+ if let Some(operation) = self.history.get(self.cursor) {
+ let task = self.undo_operation(operation, cx);
let workspace = self.workspace.clone();
cx.spawn(async move |cx| {
@@ -68,12 +88,37 @@ impl UndoManager {
}
}
+ pub fn redo(&mut self, _cx: &mut App) {
+ if self.cursor >= self.history.len() {
+ return;
+ }
+
+ if let Some(_operation) = self.history.get(self.cursor) {
+ // TODO!: Implement actual operation redo.
+ }
+
+ self.cursor += 1;
+ }
+
pub fn record(&mut self, operation: ProjectPanelOperation) {
- if self.stack.len() >= self.limit {
- self.stack.pop_front();
+ // Recording a new operation while the cursor is not at the end of the
+ // undo history should remove all operations from the cursor position to
+ // the end instead of inserting an operation in the middle of the undo
+ // history.
+ if self.cursor < self.history.len() {
+ self.history.drain(self.cursor..);
}
- self.stack.push_back(operation);
+ // The `cursor` is only increased in the case where the history's length
+ // is not yet at the limit, because when it is, the `cursor` value
+ // should already match `limit`.
+ if self.history.len() >= self.limit {
+ self.history.pop_front();
+ self.history.push_back(operation);
+ } else {
+ self.history.push_back(operation);
+ self.cursor += 1;
+ }
}
pub fn record_batch(&mut self, operations: impl IntoIterator<Item = ProjectPanelOperation>) {
@@ -92,9 +137,9 @@ impl UndoManager {
///
/// For all operations other than [`crate::undo::ProjectPanelOperation::Batch`], a maximum
/// of one error is returned.
- fn revert_operation(
+ fn undo_operation(
&self,
- operation: ProjectPanelOperation,
+ operation: &ProjectPanelOperation,
cx: &mut App,
) -> Task<Vec<anyhow::Error>> {
match operation {
@@ -167,7 +212,7 @@ impl UndoManager {
let tasks: Vec<_> = operations
.into_iter()
.rev()
- .map(|operation| self.revert_operation(operation, cx))
+ .map(|operation| self.undo_operation(operation, cx))
.collect();
cx.spawn(async move |_| {
@@ -223,7 +268,7 @@ mod test {
undo::{ProjectPanelOperation, UndoManager},
};
use gpui::{Entity, TestAppContext, VisualTestContext};
- use project::{FakeFs, Project, ProjectPath};
+ use project::{FakeFs, Project, ProjectPath, WorktreeId};
use std::sync::Arc;
use util::rel_path::rel_path;
use workspace::MultiWorkspace;
@@ -250,6 +295,15 @@ mod test {
TestContext { project, panel }
}
+ fn build_create_operation(worktree_id: WorktreeId, file_name: &str) -> ProjectPanelOperation {
+ ProjectPanelOperation::Create {
+ project_path: ProjectPath {
+ path: Arc::from(rel_path(file_name)),
+ worktree_id,
+ },
+ }
+ }
+
#[gpui::test]
async fn test_limit(cx: &mut TestAppContext) {
let test_context = init_test(cx).await;
@@ -257,21 +311,14 @@ mod test {
project.visible_worktrees(cx).next().unwrap().read(cx).id()
});
- let build_create_operation = |file_name: &str| ProjectPanelOperation::Create {
- project_path: ProjectPath {
- path: Arc::from(rel_path(file_name)),
- worktree_id,
- },
- };
-
// Since we're updating the `ProjectPanel`'s undo manager with one whose
// limit is 3 operations, we only need to create 4 operations which
// we'll record, in order to confirm that the oldest operation is
// evicted.
- let operation_a = build_create_operation("file_a.txt");
- let operation_b = build_create_operation("file_b.txt");
- let operation_c = build_create_operation("file_c.txt");
- let operation_d = build_create_operation("file_d.txt");
+ let operation_a = build_create_operation(worktree_id, "file_a.txt");
+ let operation_b = build_create_operation(worktree_id, "file_b.txt");
+ let operation_c = build_create_operation(worktree_id, "file_c.txt");
+ let operation_d = build_create_operation(worktree_id, "file_d.txt");
test_context.panel.update(cx, move |panel, _cx| {
panel.undo_manager = UndoManager::new_with_limit(panel.workspace.clone(), 3);
@@ -280,7 +327,120 @@ mod test {
panel.undo_manager.record(operation_c);
panel.undo_manager.record(operation_d);
- assert_eq!(panel.undo_manager.stack.len(), 3);
+ assert_eq!(panel.undo_manager.history.len(), 3);
+ });
+ }
+
+ #[gpui::test]
+ async fn test_cursor(cx: &mut TestAppContext) {
+ let test_context = init_test(cx).await;
+ let worktree_id = test_context.project.update(cx, |project, cx| {
+ project.visible_worktrees(cx).next().unwrap().read(cx).id()
+ });
+
+ test_context.panel.update(cx, |panel, _cx| {
+ panel.undo_manager = UndoManager::new_with_limit(panel.workspace.clone(), 3);
+ panel
+ .undo_manager
+ .record(build_create_operation(worktree_id, "file_a.txt"));
+
+ assert_eq!(panel.undo_manager.cursor, 1);
+ });
+
+ test_context.panel.update(cx, |panel, cx| {
+ panel.undo_manager.undo(cx);
+
+ // Ensure that only the `UndoManager::cursor` is updated, as the
+ // history should remain unchanged, so we can later redo the
+ // operation.
+ assert_eq!(panel.undo_manager.cursor, 0);
+ assert_eq!(
+ panel.undo_manager.history,
+ vec![build_create_operation(worktree_id, "file_a.txt")]
+ );
+
+ panel.undo_manager.undo(cx);
+
+ // Undoing when cursor is already at `0` should have no effect on
+ // both the `cursor` and `history`.
+ assert_eq!(panel.undo_manager.cursor, 0);
+ assert_eq!(
+ panel.undo_manager.history,
+ vec![build_create_operation(worktree_id, "file_a.txt")]
+ );
+ });
+
+ test_context.panel.update(cx, |panel, cx| {
+ panel.undo_manager.redo(cx);
+
+ // Ensure that only the `UndoManager::cursor` is updated, since
+ // we're only re-doing an operation that was already part of the
+ // undo history.
+ assert_eq!(panel.undo_manager.cursor, 1);
+ assert_eq!(
+ panel.undo_manager.history,
+ vec![build_create_operation(worktree_id, "file_a.txt")]
+ );
+ });
+
+ test_context.panel.update(cx, |panel, _cx| {
+ panel
+ .undo_manager
+ .record(build_create_operation(worktree_id, "file_b.txt"));
+ panel
+ .undo_manager
+ .record(build_create_operation(worktree_id, "file_c.txt"));
+
+ assert_eq!(panel.undo_manager.cursor, panel.undo_manager.limit);
+
+ panel
+ .undo_manager
+ .record(build_create_operation(worktree_id, "file_d.txt"));
+
+ // Ensure that the operation to create `file_a.txt` has been evicted
+ // but the cursor has not grown when that new operation was
+ // recorded, as the history was already at its limit.
+ assert_eq!(panel.undo_manager.cursor, panel.undo_manager.limit);
+ assert_eq!(
+ panel.undo_manager.history,
+ vec![
+ build_create_operation(worktree_id, "file_b.txt"),
+ build_create_operation(worktree_id, "file_c.txt"),
+ build_create_operation(worktree_id, "file_d.txt")
+ ]
+ );
+ });
+
+ // We'll now undo 2 operations, ensuring that the `cursor` is updated
+ // accordingly. Afterwards, we'll record a new operation and verify that
+ // the `cursor` is incremented but that all operations from the previous
+ // cursor position onwards are discarded.
+ test_context.panel.update(cx, |panel, cx| {
+ panel.undo_manager.undo(cx);
+ panel.undo_manager.undo(cx);
+
+ assert_eq!(panel.undo_manager.cursor, 1);
+ assert_eq!(
+ panel.undo_manager.history,
+ vec![
+ build_create_operation(worktree_id, "file_b.txt"),
+ build_create_operation(worktree_id, "file_c.txt"),
+ build_create_operation(worktree_id, "file_d.txt")
+ ]
+ );
+
+ panel
+ .undo_manager
+ .record(build_create_operation(worktree_id, "file_e.txt"));
+
+ assert_eq!(panel.undo_manager.cursor, 2);
+ assert_eq!(
+ panel.undo_manager.history,
+ vec![
+ build_create_operation(worktree_id, "file_b.txt"),
+ build_create_operation(worktree_id, "file_e.txt"),
+ ]
+ );
});
}
}