From e97db03b37c45cdabfcc95a70921adc3290761df Mon Sep 17 00:00:00 2001 From: dino Date: Thu, 19 Mar 2026 19:35:40 +0000 Subject: [PATCH] feat(project_panel): support redoing batch operations * Update `project_panel::undo::UndoManager::redo_operation` in order to suport redoing `ProjectPanelOperation::Batch` operations. * Introduce `project_panel::undo::test::build_rename_operation` helper function * Add tests for ensuring that redoing a batch of operations works as expected --- crates/project_panel/src/project_panel.rs | 2 - .../project_panel/src/project_panel_tests.rs | 90 ++++++++++++++++--- crates/project_panel/src/undo.rs | 81 ++++++++++++++--- 3 files changed, 149 insertions(+), 24 deletions(-) diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index f0db5c4592dab9a0440f62d3efdc325f4a47aece..6983911b427ce053b88aebf1a52a82602f6efd5c 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -401,8 +401,6 @@ 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, ] diff --git a/crates/project_panel/src/project_panel_tests.rs b/crates/project_panel/src/project_panel_tests.rs index 843e627ae39d1f59d3574168a2d4f36f180cf30d..0c0501a6a824b0b27a9f8823b09eac5bea9c5108 100644 --- a/crates/project_panel/src/project_panel_tests.rs +++ b/crates/project_panel/src/project_panel_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::undo::test::{build_create_operation, build_rename_operation}; use collections::HashSet; use editor::MultiBufferOffset; use gpui::{Empty, Entity, TestAppContext, VisualTestContext}; @@ -2519,18 +2520,8 @@ async fn test_undo_batch(cx: &mut gpui::TestAppContext) { // being provided in the operations. panel.update(cx, |panel, _cx| { panel.undo_manager.record_batch(vec![ - ProjectPanelOperation::Create { - project_path: ProjectPath { - worktree_id, - path: Arc::from(rel_path("src/main.rs")), - }, - }, - ProjectPanelOperation::Create { - project_path: ProjectPath { - worktree_id, - path: Arc::from(rel_path("src/")), - }, - }, + build_create_operation(worktree_id, "src/main.rs"), + build_create_operation(worktree_id, "src/"), ]); }); @@ -2558,6 +2549,81 @@ async fn test_undo_batch(cx: &mut gpui::TestAppContext) { ); } +#[gpui::test] +async fn test_redo_batch(cx: &mut gpui::TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/root", + json!({ + "file_c.txt": "", + "file_d.txt": "", + }), + ) + .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)); + let workspace = window + .read_with(cx, |mw, _| mw.workspace().clone()) + .unwrap(); + let cx = &mut VisualTestContext::from_window(window.into(), cx); + let panel = workspace.update_in(cx, ProjectPanel::new); + let worktree_id = project.update(cx, |project, cx| { + project.visible_worktrees(cx).next().unwrap().read(cx).id() + }); + cx.run_until_parked(); + + // At the time of writing, only the `ProjectPanelOperation::Rename` + // operation supports redoing. As such, that's what we'll use in the batch + // operation, to ensure that undoing and redoing a batch operation works as + // expected. + panel.update(cx, |panel, _cx| { + panel.undo_manager.record_batch(vec![ + build_rename_operation(worktree_id, "file_a.txt", "file_c.txt"), + build_rename_operation(worktree_id, "file_b.txt", "file_d.txt"), + ]); + }); + + // Before proceeding, ensure that both `file_c.txt` as well as `file_d.txt` + // exist in the filesystem, so we can later ensure that undoing renames + // these files and redoing renames again to how the test started. + assert_eq!( + fs.files(), + vec![ + PathBuf::from(path!("/root/file_c.txt")), + PathBuf::from(path!("/root/file_d.txt")) + ] + ); + + panel.update_in(cx, |panel, window, cx| { + panel.undo(&Undo, window, cx); + }); + cx.run_until_parked(); + + assert_eq!( + fs.files(), + vec![ + PathBuf::from(path!("/root/file_a.txt")), + PathBuf::from(path!("/root/file_b.txt")) + ] + ); + + panel.update_in(cx, |panel, window, cx| { + panel.redo(&Redo, window, cx); + }); + cx.run_until_parked(); + + assert_eq!( + fs.files(), + vec![ + PathBuf::from(path!("/root/file_c.txt")), + PathBuf::from(path!("/root/file_d.txt")) + ] + ); +} + #[gpui::test] async fn test_paste_external_paths(cx: &mut gpui::TestAppContext) { init_test(cx); diff --git a/crates/project_panel/src/undo.rs b/crates/project_panel/src/undo.rs index 624caa68d2244f4359f88898ee8966c644a2740c..c60b90837504af1b523d1e63b5f782f542cd63d8 100644 --- a/crates/project_panel/src/undo.rs +++ b/crates/project_panel/src/undo.rs @@ -135,7 +135,30 @@ impl UndoManager { ProjectPanelOperation::Rename { old_path, new_path } => { self.rename(old_path, new_path, cx) } - _ => Task::ready(vec![anyhow!("Not implemented.")]), + 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." + )]), } } @@ -231,13 +254,7 @@ impl UndoManager { .map(|operation| self.undo_operation(operation, cx)) .collect(); - cx.spawn(async move |_| { - let mut errors = Vec::new(); - for task in tasks { - errors.extend(task.await); - } - errors - }) + Self::run_sequentially(tasks, cx) } } } @@ -274,6 +291,27 @@ impl UndoManager { }) } + /// 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>>, + cx: &mut App, + ) -> Task> { + cx.spawn(async move |_| { + let mut errors = Vec::new(); + + for task in tasks { + errors.extend(task.await); + } + + errors + }) + } + /// 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 @@ -314,7 +352,7 @@ impl UndoManager { } #[cfg(test)] -mod test { +pub(crate) mod test { use crate::{ ProjectPanel, project_panel_tests, undo::{ProjectPanelOperation, UndoManager}, @@ -347,7 +385,10 @@ mod test { TestContext { project, panel } } - fn build_create_operation(worktree_id: WorktreeId, file_name: &str) -> ProjectPanelOperation { + pub(crate) fn build_create_operation( + worktree_id: WorktreeId, + file_name: &str, + ) -> ProjectPanelOperation { ProjectPanelOperation::Create { project_path: ProjectPath { path: Arc::from(rel_path(file_name)), @@ -356,6 +397,26 @@ mod test { } } + pub(crate) fn build_rename_operation( + worktree_id: WorktreeId, + from: &str, + to: &str, + ) -> ProjectPanelOperation { + let from_path = Arc::from(rel_path(from)); + let to_path = Arc::from(rel_path(to)); + + ProjectPanelOperation::Rename { + old_path: ProjectPath { + worktree_id, + path: from_path, + }, + new_path: ProjectPath { + worktree_id, + path: to_path, + }, + } + } + #[gpui::test] async fn test_limit(cx: &mut TestAppContext) { let test_context = init_test(cx).await;