diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index 8671bb7be912b9ed89851f00aaa68ecb8af8ca56..6d2744a6cae856dc0e4b30dff17827e86dcf1d42 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -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 }], diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index 26848eeed695e00b91e1f52015e7a11a1b8a03ed..3cd9f1eea542ef093719f06c5de54f922a6fbc04 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -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 }], diff --git a/assets/keymaps/default-windows.json b/assets/keymaps/default-windows.json index dcfee8ec86bcbe7cf54f4ccbf627de1d1f66cbe9..0ee9bcfbb45a13aacee7359527fb8c3929a1412f 100644 --- a/assets/keymaps/default-windows.json +++ b/assets/keymaps/default-windows.json @@ -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 }], diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 41acd58c3cd2fb06ac68d1673a6c9fb21bc46bb5..f0db5c4592dab9a0440f62d3efdc325f4a47aece 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -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.undo_manager.redo(cx); + cx.notify(); + } + fn rename_impl( &mut self, selection: Option>, @@ -6684,6 +6698,7 @@ impl Render for ProjectPanel { .on_action(cx.listener(Self::compare_marked_files)) .when(cx.has_flag::(), |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)) diff --git a/crates/project_panel/src/undo.rs b/crates/project_panel/src/undo.rs index 3a8baa23c55db8f3572174ee667196936e633281..c9384072e302d22fe2cd5d6ffc3358ad39f61d23 100644 --- a/crates/project_panel/src/undo.rs +++ b/crates/project_panel/src/undo.rs @@ -10,7 +10,7 @@ use workspace::{ const MAX_UNDO_OPERATIONS: usize = 10_000; -#[derive(Clone)] +#[derive(Clone, Debug, PartialEq)] pub enum ProjectPanelOperation { Batch(Vec), Create { @@ -24,8 +24,13 @@ pub enum ProjectPanelOperation { pub struct UndoManager { workspace: WeakEntity, - stack: VecDeque, - /// Maximum number of operations to keep on the undo stack. + history: VecDeque, + /// 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) { @@ -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> { 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"), + ] + ); }); } }