diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 993ae13650d2309fe78624b29e39f72f00e34c31..f45d637e52d5c04516249e95744c7dc6bb523fb2 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -902,7 +902,7 @@ impl ProjectPanel { unfolded_dir_ids: Default::default(), }, update_visible_entries_task: Default::default(), - undo_manager: UndoManager::new(workspace.weak_handle()), + undo_manager: UndoManager::new(workspace.weak_handle(), cx.weak_entity()), }; this.update_visible_entries(None, false, false, window, cx); diff --git a/crates/project_panel/src/project_panel_tests.rs b/crates/project_panel/src/project_panel_tests.rs index 0c0501a6a824b0b27a9f8823b09eac5bea9c5108..ae155d8bf13242b1d66f1d2a3107369f4705c88d 100644 --- a/crates/project_panel/src/project_panel_tests.rs +++ b/crates/project_panel/src/project_panel_tests.rs @@ -9637,7 +9637,7 @@ async fn test_hide_hidden_entries(cx: &mut gpui::TestAppContext) { ); } -fn select_path(panel: &Entity, path: &str, cx: &mut VisualTestContext) { +pub(crate) fn select_path(panel: &Entity, path: &str, cx: &mut VisualTestContext) { let path = rel_path(path); panel.update_in(cx, |panel, window, cx| { for worktree in panel.project.read(cx).worktrees(cx).collect::>() { diff --git a/crates/project_panel/src/undo.rs b/crates/project_panel/src/undo.rs index 0f5a062380c904a3f99bcb670e9e8d6e9f46af5f..32ad8d8ed06576dacab9eb6ad71508051826cbc0 100644 --- a/crates/project_panel/src/undo.rs +++ b/crates/project_panel/src/undo.rs @@ -1,8 +1,9 @@ -use anyhow::anyhow; +use crate::ProjectPanel; +use anyhow::{Result, anyhow}; use gpui::{AppContext, SharedString, Task, WeakEntity}; use project::ProjectPath; use std::collections::VecDeque; -use ui::{App, IntoElement, Label, ParentElement, Styled, v_flex}; +use ui::App; use workspace::{ Workspace, notifications::{NotificationId, simple_message_notification::MessageNotification}, @@ -14,169 +15,143 @@ const MAX_UNDO_OPERATIONS: usize = 10_000; pub enum ProjectPanelOperation { Batch(Vec), Create { project_path: ProjectPath }, + Trash { project_path: ProjectPath }, Rename { from: ProjectPath, to: ProjectPath }, } +impl ProjectPanelOperation { + fn inverse(&self) -> Self { + match self { + Self::Create { project_path } => Self::Trash { + project_path: project_path.clone(), + }, + Self::Trash { project_path } => Self::Create { + project_path: project_path.clone(), + }, + Self::Rename { from, to } => Self::Rename { + from: to.clone(), + to: from.clone(), + }, + // When inverting a batch of operations, we reverse the order of + // operations to handle dependencies between them. For example, if a + // batch contains the following order of operations: + // + // 1. Create `src/` + // 2. Create `src/main.rs` + // + // If we first tried to revert the directory creation, it would fail + // because there's still files inside the directory. + Self::Batch(operations) => Self::Batch( + operations + .iter() + .rev() + .map(|operation| operation.inverse()) + .collect(), + ), + } + } +} + pub struct UndoManager { workspace: WeakEntity, - 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, + panel: WeakEntity, + undo_stack: VecDeque, + redo_stack: Vec, /// Maximum number of operations to keep on the undo history. limit: usize, } impl UndoManager { - pub fn new(workspace: WeakEntity) -> Self { - Self::new_with_limit(workspace, MAX_UNDO_OPERATIONS) + pub fn new(workspace: WeakEntity, panel: WeakEntity) -> Self { + Self::new_with_limit(workspace, panel, MAX_UNDO_OPERATIONS) } - pub fn new_with_limit(workspace: WeakEntity, limit: usize) -> Self { + pub fn new_with_limit( + workspace: WeakEntity, + panel: WeakEntity, + limit: usize, + ) -> Self { Self { workspace, + panel, limit, - cursor: 0, - history: VecDeque::new(), + undo_stack: VecDeque::new(), + redo_stack: Vec::new(), } } pub fn can_undo(&self) -> bool { - self.cursor > 0 + !self.undo_stack.is_empty() } pub fn can_redo(&self) -> bool { - self.cursor < self.history.len() + !self.redo_stack.is_empty() } pub fn undo(&mut self, cx: &mut App) { - 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); + if let Some(operation) = self.undo_stack.pop_back() { + let task = self.execute_operation(&operation, cx); + let panel = self.panel.clone(); let workspace = self.workspace.clone(); - cx.spawn(async move |cx| { - let errors = task.await; - if !errors.is_empty() { - cx.update(|cx| { - let messages = errors - .iter() - .map(|err| SharedString::from(err.to_string())) - .collect(); - - Self::show_errors( - "Failed to undo Project Panel Operation(s)", - workspace, - messages, - cx, - ) - }) - } + cx.spawn(async move |cx| match task.await { + Ok(operation) => panel.update(cx, |panel, _cx| { + panel.undo_manager.redo_stack.push(operation) + }), + Err(err) => cx.update(|cx| { + Self::show_error( + "Failed to undo Project Panel Operation(s)", + workspace, + err.to_string().into(), + cx, + ); + + Ok(()) + }), }) .detach(); } } pub fn redo(&mut self, cx: &mut App) { - if self.cursor >= self.history.len() { - return; - } - - if let Some(operation) = self.history.get(self.cursor) { - let task = self.redo_operation(operation, cx); + if let Some(operation) = self.redo_stack.pop() { + let task = self.execute_operation(&operation, cx); + let panel = self.panel.clone(); let workspace = self.workspace.clone(); - cx.spawn(async move |cx| { - let errors = task.await; - if !errors.is_empty() { - cx.update(|cx| { - let messages = errors - .iter() - .map(|err| SharedString::from(err.to_string())) - .collect(); - - Self::show_errors( - "Failed to redo Project Panel Operation(s)", - workspace, - messages, - cx, - ) - }) - } + cx.spawn(async move |cx| match task.await { + Ok(operation) => panel.update(cx, |panel, _cx| { + panel.undo_manager.undo_stack.push_back(operation) + }), + Err(err) => cx.update(|cx| { + Self::show_error( + "Failed to redo Project Panel Operation(s)", + workspace, + err.to_string().into(), + cx, + ); + + Ok(()) + }), }) .detach(); } - - self.cursor += 1; - } - - fn redo_operation( - &self, - operation: &ProjectPanelOperation, - cx: &mut App, - ) -> Task> { - match operation { - ProjectPanelOperation::Rename { - from: old_path, - to: new_path, - } => self.rename(old_path, new_path, cx), - ProjectPanelOperation::Batch(operations) => { - // Ensure that, when redoing a batch of operations, we do these - // in the same order as they were passed to the batch, as there - // might be dependencies between them. - // - // For example, imagine the following batch of operations: - // - // 1. Create `src/` - // 2. Create `src/main.rs` - // - // If these are not done in order, there's no guarantee that the - // second one succeeds, as the command to redo the `src/main.rs` - // file creation might fail unless it also forces the parent - // directory to be created first. - let tasks: Vec<_> = operations - .into_iter() - .map(|operation| self.redo_operation(operation, cx)) - .collect(); - - Self::run_sequentially(tasks, cx) - } - ProjectPanelOperation::Create { .. } => Task::ready(vec![anyhow!( - "Redoing create operations is currently not supported." - )]), - } } pub fn record(&mut self, operation: ProjectPanelOperation) { - // 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..); + // Recording a new operation while there's still operations in the + // `redo_stack` should clear all operations from the `redo_stack`, as we + // might end up in a situation where the state diverges and the + // `redo_stack` operations can no longer be done. + if !self.redo_stack.is_empty() { + self.redo_stack.clear(); } - // 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; + if self.undo_stack.len() >= self.limit { + self.undo_stack.pop_front(); } + + self.undo_stack.push_back(operation.inverse()); } pub fn record_batch(&mut self, operations: impl IntoIterator) { @@ -190,69 +165,18 @@ impl UndoManager { self.record(operation); } - /// Attempts to revert the provided `operation`, returning a vector of errors - /// in case there was any failure while reverting the operation. - /// - /// For all operations other than [`crate::undo::ProjectPanelOperation::Batch`], a maximum - /// of one error is returned. - fn undo_operation( - &self, + /// Attempts to execute the provided operation, returning the inverse of the + /// provided `operation` as a result. + fn execute_operation( + &mut self, operation: &ProjectPanelOperation, cx: &mut App, - ) -> Task> { + ) -> Task> { match operation { - ProjectPanelOperation::Create { project_path } => { - let Some(workspace) = self.workspace.upgrade() else { - return Task::ready(vec![anyhow!("Failed to obtain workspace.")]); - }; - - let result = workspace.update(cx, |workspace, cx| { - workspace.project().update(cx, |project, cx| { - let entry_id = project - .entry_for_path(&project_path, cx) - .map(|entry| entry.id) - .ok_or_else(|| anyhow!("No entry for path."))?; - - project - .delete_entry(entry_id, true, cx) - .ok_or_else(|| anyhow!("Failed to trash entry.")) - }) - }); - - let task = match result { - Ok(task) => task, - Err(err) => return Task::ready(vec![err]), - }; - - cx.spawn(async move |_| match task.await { - Ok(_) => vec![], - Err(err) => vec![err], - }) - } - ProjectPanelOperation::Rename { - from: old_path, - to: new_path, - } => self.rename(new_path, old_path, cx), - ProjectPanelOperation::Batch(operations) => { - // When reverting operations in a batch, we reverse the order of - // operations to handle dependencies between them. For example, - // if a batch contains the following order of operations: - // - // 1. Create `src/` - // 2. Create `src/main.rs` - // - // If we first try to revert the directory creation, it would - // fail because there's still files inside the directory. - // Operations are also reverted sequentially in order to avoid - // this same problem. - let tasks: Vec<_> = operations - .into_iter() - .rev() - .map(|operation| self.undo_operation(operation, cx)) - .collect(); - - Self::run_sequentially(tasks, cx) - } + ProjectPanelOperation::Rename { from, to } => self.rename(from, to, cx), + ProjectPanelOperation::Trash { project_path } => self.trash(project_path, cx), + ProjectPanelOperation::Create { project_path } => self.create(project_path, cx), + ProjectPanelOperation::Batch(operations) => self.batch(operations, cx), } } @@ -261,9 +185,9 @@ impl UndoManager { from: &ProjectPath, to: &ProjectPath, cx: &mut App, - ) -> Task> { + ) -> Task> { let Some(workspace) = self.workspace.upgrade() else { - return Task::ready(vec![anyhow!("Failed to obtain workspace.")]); + return Task::ready(Err(anyhow!("Failed to obtain workspace."))); }; let result = workspace.update(cx, |workspace, cx| { @@ -279,44 +203,118 @@ impl UndoManager { let task = match result { Ok(task) => task, - Err(err) => return Task::ready(vec![err]), + Err(err) => return Task::ready(Err(err)), }; + let from = from.clone(); + let to = to.clone(); cx.spawn(async move |_| match task.await { - Ok(_) => vec![], - Err(err) => vec![err], + Err(err) => Err(err), + Ok(_) => Ok(ProjectPanelOperation::Rename { + from: to.clone(), + to: from.clone(), + }), }) } - /// Awaits each task in `tasks` sequentially, collecting any errors. - /// - /// Sequential execution is important for [`ProjectPanelOperation::Batch`] - /// operations, where there may be ordering dependencies between tasks. For - /// example, a directory must be created before files can be placed inside - /// it. - fn run_sequentially( - tasks: Vec>>, + fn create( + &self, + project_path: &ProjectPath, cx: &mut App, - ) -> Task> { + ) -> Task> { + let Some(workspace) = self.workspace.upgrade() else { + return Task::ready(Err(anyhow!("Failed to obtain workspace."))); + }; + + let task = workspace.update(cx, |workspace, cx| { + workspace.project().update(cx, |project, cx| { + // This should not be hardcoded to `false`, as it can genuinely + // be a directory and it misses all the nuances and details from + // `ProjectPanel::confirm_edit`. However, we expect this to be a + // short-lived solution as we add support for restoring trashed + // files, at which point we'll no longer need to `Create` new + // files, any redoing of a trash operation should be a restore. + let is_directory = false; + project.create_entry(project_path.clone(), is_directory, cx) + }) + }); + + let project_path = project_path.clone(); + cx.spawn(async move |_| match task.await { + Ok(_) => Ok(ProjectPanelOperation::Trash { project_path }), + Err(err) => Err(err), + }) + } + + fn trash( + &self, + project_path: &ProjectPath, + cx: &mut App, + ) -> Task> { + let Some(workspace) = self.workspace.upgrade() else { + return Task::ready(Err(anyhow!("Failed to obtain workspace."))); + }; + + let result = workspace.update(cx, |workspace, cx| { + workspace.project().update(cx, |project, cx| { + let entry_id = project + .entry_for_path(&project_path, cx) + .map(|entry| entry.id) + .ok_or_else(|| anyhow!("No entry for path."))?; + + project + .delete_entry(entry_id, true, cx) + .ok_or_else(|| anyhow!("Failed to trash entry.")) + }) + }); + + let task = match result { + Ok(task) => task, + Err(err) => return Task::ready(Err(err)), + }; + + let project_path = project_path.clone(); + cx.spawn(async move |_| match task.await { + // We'll want this to eventually be a `Restore` operation, once + // we've added support, in `fs` to track and restore a trashed file. + Ok(_) => Ok(ProjectPanelOperation::Create { project_path }), + Err(err) => Err(err), + }) + } + + fn batch( + &mut self, + operations: &[ProjectPanelOperation], + cx: &mut App, + ) -> Task> { + let tasks: Vec<_> = operations + .into_iter() + .map(|operation| self.execute_operation(operation, cx)) + .collect(); + cx.spawn(async move |_| { - let mut errors = Vec::new(); + let mut operations = Vec::new(); for task in tasks { - errors.extend(task.await); + match task.await { + Ok(operation) => operations.push(operation), + Err(err) => return Err(err), + } } - errors + // Return the `ProjectPanelOperation::Batch` that reverses all of + // the provided operations. The order of operations should be reversed + // so that dependencies are handled correctly. + operations.reverse(); + Ok(ProjectPanelOperation::Batch(operations)) }) } - /// Displays a notification with the list of provided errors ensuring that, - /// when more than one error is provided, which can be the case when dealing - /// with undoing a [`crate::undo::ProjectPanelOperation::Batch`], a list is - /// displayed with each of the errors, instead of a single message. - fn show_errors( + /// Displays a notification with the provided `title` and `error`. + fn show_error( title: impl Into, workspace: WeakEntity, - messages: Vec, + error: SharedString, cx: &mut App, ) { workspace @@ -325,23 +323,7 @@ impl UndoManager { NotificationId::Named(SharedString::new_static("project_panel_undo")); workspace.show_notification(notification_id, cx, move |cx| { - cx.new(|cx| { - if let [err] = messages.as_slice() { - MessageNotification::new(err.to_string(), cx).with_title(title) - } else { - MessageNotification::new_from_builder(cx, move |_, _| { - v_flex() - .gap_1() - .children( - messages - .iter() - .map(|message| Label::new(format!("- {message}"))), - ) - .into_any_element() - }) - .with_title(title) - } - }) + cx.new(|cx| MessageNotification::new(error.to_string(), cx).with_title(title)) }) }) .ok(); @@ -354,8 +336,9 @@ pub(crate) mod test { ProjectPanel, project_panel_tests, undo::{ProjectPanelOperation, UndoManager}, }; - use gpui::{Entity, TestAppContext, VisualTestContext}; + use gpui::{Entity, TestAppContext, VisualTestContext, WindowHandle}; use project::{FakeFs, Project, ProjectPath, WorktreeId}; + use serde_json::{Value, json}; use std::sync::Arc; use util::rel_path::rel_path; use workspace::MultiWorkspace; @@ -363,12 +346,16 @@ pub(crate) mod test { struct TestContext { project: Entity, panel: Entity, + window: WindowHandle, } - async fn init_test(cx: &mut TestAppContext) -> TestContext { + async fn init_test(cx: &mut TestAppContext, tree: Option) -> TestContext { project_panel_tests::init_test(cx); let fs = FakeFs::new(cx.executor()); + if let Some(tree) = tree { + fs.insert_tree("/root", tree).await; + } let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await; let window = cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); @@ -379,7 +366,11 @@ pub(crate) mod test { let panel = workspace.update_in(cx, ProjectPanel::new); cx.run_until_parked(); - TestContext { project, panel } + TestContext { + project, + panel, + window, + } } pub(crate) fn build_create_operation( @@ -394,6 +385,18 @@ pub(crate) mod test { } } + pub(crate) fn build_trash_operation( + worktree_id: WorktreeId, + file_name: &str, + ) -> ProjectPanelOperation { + ProjectPanelOperation::Trash { + project_path: ProjectPath { + path: Arc::from(rel_path(file_name)), + worktree_id, + }, + } + } + pub(crate) fn build_rename_operation( worktree_id: WorktreeId, from: &str, @@ -414,9 +417,33 @@ pub(crate) mod test { } } + async fn rename( + panel: &Entity, + from: &str, + to: &str, + cx: &mut VisualTestContext, + ) { + project_panel_tests::select_path(panel, from, cx); + panel.update_in(cx, |panel, window, cx| { + panel.rename(&Default::default(), window, cx) + }); + cx.run_until_parked(); + + panel + .update_in(cx, |panel, window, cx| { + panel + .filename_editor + .update(cx, |editor, cx| editor.set_text(to, window, cx)); + panel.confirm_edit(true, window, cx).unwrap() + }) + .await + .unwrap(); + cx.run_until_parked(); + } + #[gpui::test] async fn test_limit(cx: &mut TestAppContext) { - let test_context = init_test(cx).await; + let test_context = init_test(cx, None).await; let worktree_id = test_context.project.update(cx, |project, cx| { project.visible_worktrees(cx).next().unwrap().read(cx).id() }); @@ -430,127 +457,216 @@ pub(crate) mod test { 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); + test_context.panel.update(cx, move |panel, cx| { + panel.undo_manager = + UndoManager::new_with_limit(panel.workspace.clone(), cx.weak_entity(), 3); panel.undo_manager.record(operation_a); panel.undo_manager.record(operation_b); panel.undo_manager.record(operation_c); panel.undo_manager.record(operation_d); - assert_eq!(panel.undo_manager.history.len(), 3); + assert_eq!(panel.undo_manager.undo_stack.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| { + async fn test_undo_redo_stacks(cx: &mut TestAppContext) { + let TestContext { + window, + panel, + project, + .. + } = init_test( + cx, + Some(json!({ + "a.txt": "", + "b.txt": "" + })), + ) + .await; + let worktree_id = project.update(cx, |project, cx| { project.visible_worktrees(cx).next().unwrap().read(cx).id() }); + let cx = &mut VisualTestContext::from_window(window.into(), cx); - 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); + // Start by renaming `src/file_a.txt` to `src/file_1.txt` and asserting + // we get the correct inverse operation in the + // `UndoManager::undo_stackand asserting we get the correct inverse + // operation in the `UndoManager::undo_stack`. + rename(&panel, "root/a.txt", "1.txt", cx).await; + panel.update(cx, |panel, _cx| { + assert_eq!( + panel.undo_manager.undo_stack, + vec![build_rename_operation(worktree_id, "1.txt", "a.txt")] + ); + assert!(panel.undo_manager.redo_stack.is_empty()); }); - test_context.panel.update(cx, |panel, cx| { - panel.undo_manager.undo(cx); + // After undoing, the operation to be executed should be popped from + // `UndoManager::undo_stack` and its inverse operation pushed to + // `UndoManager::redo_stack`. + panel.update_in(cx, |panel, window, cx| { + panel.undo(&Default::default(), window, cx); + }); + cx.run_until_parked(); - // 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); + panel.update(cx, |panel, _cx| { + assert!(panel.undo_manager.undo_stack.is_empty()); assert_eq!( - panel.undo_manager.history, - vec![build_create_operation(worktree_id, "file_a.txt")] + panel.undo_manager.redo_stack, + vec![build_rename_operation(worktree_id, "a.txt", "1.txt")] ); + }); - panel.undo_manager.undo(cx); + // Redoing should have the same effect as undoing, but in reverse. + panel.update_in(cx, |panel, window, cx| { + panel.redo(&Default::default(), window, cx); + }); + cx.run_until_parked(); - // Undoing when cursor is already at `0` should have no effect on - // both the `cursor` and `history`. - assert_eq!(panel.undo_manager.cursor, 0); + panel.update(cx, |panel, _cx| { assert_eq!( - panel.undo_manager.history, - vec![build_create_operation(worktree_id, "file_a.txt")] + panel.undo_manager.undo_stack, + vec![build_rename_operation(worktree_id, "1.txt", "a.txt")] ); + assert!(panel.undo_manager.redo_stack.is_empty()); + }); + } + + #[gpui::test] + async fn test_undo_redo_trash(cx: &mut TestAppContext) { + let TestContext { + window, + panel, + project, + .. + } = init_test( + cx, + Some(json!({ + "a.txt": "", + "b.txt": "" + })), + ) + .await; + let worktree_id = project.update(cx, |project, cx| { + project.visible_worktrees(cx).next().unwrap().read(cx).id() + }); + let cx = &mut VisualTestContext::from_window(window.into(), cx); + + // Start by setting up the `UndoManager::undo_stack` such that, undoing + // the last user operation will trash `a.txt`. + panel.update(cx, |panel, _cx| { + panel + .undo_manager + .undo_stack + .push_back(build_trash_operation(worktree_id, "a.txt")); }); - test_context.panel.update(cx, |panel, cx| { - panel.undo_manager.redo(cx); + // Undoing should now delete the file and update the + // `UndoManager::redo_stack` state with a new `Create` operation. + panel.update_in(cx, |panel, window, cx| { + panel.undo(&Default::default(), window, cx); + }); + cx.run_until_parked(); - // 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); + panel.update(cx, |panel, _cx| { + assert!(panel.undo_manager.undo_stack.is_empty()); assert_eq!( - panel.undo_manager.history, - vec![build_create_operation(worktree_id, "file_a.txt")] + panel.undo_manager.redo_stack, + vec![build_create_operation(worktree_id, "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")); + // Redoing should create the file again and pop the operation from + // `UndoManager::redo_stack`. + panel.update_in(cx, |panel, window, cx| { + panel.redo(&Default::default(), window, cx); + }); + cx.run_until_parked(); - assert_eq!(panel.undo_manager.cursor, panel.undo_manager.limit); + panel.update(cx, |panel, _cx| { + assert_eq!( + panel.undo_manager.undo_stack, + vec![build_trash_operation(worktree_id, "a.txt")] + ); + assert!(panel.undo_manager.redo_stack.is_empty()); + }); + } - panel - .undo_manager - .record(build_create_operation(worktree_id, "file_d.txt")); + #[gpui::test] + async fn test_undo_redo_batch(cx: &mut TestAppContext) { + let TestContext { + window, + panel, + project, + .. + } = init_test( + cx, + Some(json!({ + "a.txt": "", + "b.txt": "" + })), + ) + .await; + let worktree_id = project.update(cx, |project, cx| { + project.visible_worktrees(cx).next().unwrap().read(cx).id() + }); + let cx = &mut VisualTestContext::from_window(window.into(), cx); + + // There's currently no way to trigger two file renames in a single + // operation using the `ProjectPanel`. As such, we'll directly record + // the batch of operations in `UndoManager`, simulating that `1.txt` and + // `2.txt` had been renamed to `a.txt` and `b.txt`, respectively. + panel.update(cx, |panel, _cx| { + panel.undo_manager.record_batch(vec![ + build_rename_operation(worktree_id, "1.txt", "a.txt"), + build_rename_operation(worktree_id, "2.txt", "b.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") - ] + panel.undo_manager.undo_stack, + vec![ProjectPanelOperation::Batch(vec![ + build_rename_operation(worktree_id, "b.txt", "2.txt"), + build_rename_operation(worktree_id, "a.txt", "1.txt"), + ])] ); + assert!(panel.undo_manager.redo_stack.is_empty()); }); - // 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); + panel.update_in(cx, |panel, window, cx| { + panel.undo(&Default::default(), window, cx); + }); + cx.run_until_parked(); - assert_eq!(panel.undo_manager.cursor, 1); + // Since the operations in the `Batch` are meant to be done in order, + // the inverse should have the operations in the opposite order to avoid + // dependencies. For example, creating a `src/` folder come before + // creating the `src/file_a.txt` file, but when undoing, the file should + // be trashed first. + panel.update(cx, |panel, _cx| { + assert!(panel.undo_manager.undo_stack.is_empty()); 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.redo_stack, + vec![ProjectPanelOperation::Batch(vec![ + build_rename_operation(worktree_id, "1.txt", "a.txt"), + build_rename_operation(worktree_id, "2.txt", "b.txt"), + ])] ); + }); - panel - .undo_manager - .record(build_create_operation(worktree_id, "file_e.txt")); + panel.update_in(cx, |panel, window, cx| { + panel.redo(&Default::default(), window, cx); + }); + cx.run_until_parked(); - assert_eq!(panel.undo_manager.cursor, 2); + panel.update(cx, |panel, _cx| { assert_eq!( - panel.undo_manager.history, - vec![ - build_create_operation(worktree_id, "file_b.txt"), - build_create_operation(worktree_id, "file_e.txt"), - ] + panel.undo_manager.undo_stack, + vec![ProjectPanelOperation::Batch(vec![ + build_rename_operation(worktree_id, "b.txt", "2.txt"), + build_rename_operation(worktree_id, "a.txt", "1.txt"), + ])] ); + assert!(panel.undo_manager.redo_stack.is_empty()); }); } }