dino created
* 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
expectedcrates/project_panel/src/project_panel.rs | 2
crates/project_panel/src/project_panel_tests.rs | 90 ++++++++++++++++--
crates/project_panel/src/undo.rs | 81 ++++++++++++++--
3 files changed, 149 insertions(+), 24 deletions(-)
@@ -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,
]
@@ -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);
@@ -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<Task<Vec<anyhow::Error>>>,
+ cx: &mut App,
+ ) -> Task<Vec<anyhow::Error>> {
+ 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;