diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 675d023814..5b35531fa2 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -1937,8 +1937,9 @@ impl ProjectPanel { let edit_task; let edited_entry_id; - let edited_entry; let new_project_path: ProjectPath; + let changes: Vec; + if is_new_entry { self.selection = Some(SelectedEntry { worktree_id, @@ -1949,12 +1950,12 @@ impl ProjectPanel { return None; } - edited_entry = None; edited_entry_id = NEW_ENTRY_ID; new_project_path = (worktree_id, new_path).into(); edit_task = self.project.update(cx, |project, cx| { project.create_entry(new_project_path.clone(), is_dir, cx) }); + changes = vec![Change::Created(new_project_path)]; } else { let new_path = if let Some(parent) = entry.path.parent() { parent.join(&filename).into() @@ -1968,11 +1969,31 @@ impl ProjectPanel { return None; } edited_entry_id = entry.id; - edited_entry = Some(entry); new_project_path = (worktree_id, new_path).into(); + + // Before renaming, keep track of which directories will need to be + // created, so we can remove these when undoing. + let created_dirs = crate::undo::missing_parent_dirs( + worktree.read(cx), + worktree_id, + new_project_path.path.as_ref(), + ); + edit_task = self.project.update(cx, |project, cx| { project.rename_entry(edited_entry_id, new_project_path.clone(), cx) - }) + }); + + // Record the directory creations shallowest-first, then the rename, + // so undoing reverses them in the right order. + changes = created_dirs + .into_iter() + .rev() + .map(Change::DirCreated) + .chain(std::iter::once(Change::Renamed( + (worktree_id, entry.path).into(), + new_project_path, + ))) + .collect(); }; if refocus { @@ -2008,14 +2029,7 @@ impl ProjectPanel { // changes in excluded paths, but that would mean updating // methods that rely on `EntryId` to now rely on the actual // paths. - let operation = match edited_entry { - Some(old_entry) => { - let project_path = (worktree_id, old_entry.path).into(); - Change::Renamed(project_path, new_project_path) - } - None => Change::Created(new_project_path), - }; - project_panel.undo_manager.record([operation]).log_err(); + project_panel.undo_manager.record(changes).log_err(); if let Some(selection) = &mut project_panel.selection && selection.entry_id == edited_entry_id diff --git a/crates/project_panel/src/tests/undo.rs b/crates/project_panel/src/tests/undo.rs index 918258d69a..28acd80ca1 100644 --- a/crates/project_panel/src/tests/undo.rs +++ b/crates/project_panel/src/tests/undo.rs @@ -5,11 +5,12 @@ use collections::HashSet; use editor::Editor; use fs::{FakeFs, Fs}; use gpui::{App, BorrowAppContext, Context, Entity, VisualTestContext, Window}; -use project::Project; +use project::{Project, ProjectPath}; use serde_json::{Value, json}; use settings::{SettingsStore, SplicingVec}; use std::path::Path; use std::sync::Arc; +use util::rel_path::rel_path; use workspace::{Item, MultiWorkspace, register_project_item}; use crate::project_panel_tests::{self, TestProjectItemView, find_project_entry, select_path}; @@ -326,6 +327,57 @@ async fn rename_undo_redo(cx: &mut gpui::TestAppContext) { cx.assert_fs_state_is(&["b.txt", "renamed.txt"]); } +#[gpui::test] +async fn rename_with_dir_undo_redo(cx: &mut gpui::TestAppContext) { + let mut cx = TestContext::new(cx).await; + + // Renaming a file like `a.txt` to `files/a.txt` will create the `files` + // directory, in case it doesn't exist yet. + cx.rename("a.txt", "files/nested/a.txt").await; + cx.assert_fs_state_is(&["b.txt", "files/", "files/nested/", "files/nested/a.txt"]); + + // Undoing the rename operation should also delete the `files` directory, as + // it was created specifically for the rename operation. + cx.undo().await; + cx.assert_fs_state_is(&["a.txt", "b.txt"]); + + // Redoing the rename operation should recreate the `files` directory and + // move `a.txt` back into it. + cx.redo().await; + cx.assert_fs_state_is(&["b.txt", "files/", "files/nested/", "files/nested/a.txt"]); + + // Lastly, let's insert a different file into the `files` directory to + // ensure that, when undoing the `Rename` operation, we don't delete the + // directory, as it is not empty. + // The file will be created directly through the `Project` layer instead of + // the `ProjectPanel`, to ensure that it doesn't get recorded and, when undo + // is called, we're undoing the rename. + cx.panel + .update(&mut cx.cx, |panel, cx| { + panel.project.update(cx, |project, cx| { + let worktree_id = project + .worktrees(cx) + .next() + .expect("project should have a worktree") + .read(cx) + .id(); + + let project_path = ProjectPath { + worktree_id, + path: rel_path("files/external.txt").into(), + }; + + project.create_entry(project_path, false, cx) + }) + }) + .await + .unwrap(); + cx.cx.run_until_parked(); + + cx.undo().await; + cx.assert_fs_state_is(&["a.txt", "b.txt", "files/", "files/external.txt"]); +} + #[gpui::test] async fn create_undo_redo(cx: &mut gpui::TestAppContext) { let mut cx = TestContext::new(cx).await; diff --git a/crates/project_panel/src/undo.rs b/crates/project_panel/src/undo.rs index 3bc552b7cf..8de6f77807 100644 --- a/crates/project_panel/src/undo.rs +++ b/crates/project_panel/src/undo.rs @@ -150,13 +150,20 @@ use workspace::{ NotificationId, markdown_style, simple_message_notification::MessageNotification, }, }; -use worktree::CreatedEntry; +use worktree::{CreatedEntry, Worktree}; enum Operation { Trash(ProjectPath), Rename(ProjectPath, ProjectPath), Restore(WorktreeId, TrashId), Batch(Vec), + /// Creates an empty directory, idempotently. + /// Meant to only be used when decomposing a [`Self::Rename`] that has to + /// create parent directories. + CreateDir(ProjectPath), + /// Removes a directory, but only if it is empty. + /// Meant to only be used when decomposing a [`Self::Rename`]. + RemoveDir(ProjectPath), } impl Operation { @@ -236,6 +243,14 @@ impl Operation { Change::Batched(changes) } + Operation::CreateDir(path) => { + undo_manager.create_dir(&path, cx).await?; + Change::DirCreated(path) + } + Operation::RemoveDir(path) => { + undo_manager.remove_dir(&path, cx).await?; + Change::DirRemoved(path) + } }; Ok(change) @@ -243,13 +258,13 @@ impl Operation { fn trash_paths<'a>(&'a self, paths: &mut Vec<&'a ProjectPath>) { match self { - Operation::Trash(path) => paths.push(path), - Operation::Batch(operations) => { + Self::Trash(path) => paths.push(path), + Self::Batch(operations) => { for operation in operations { operation.trash_paths(paths); } } - Operation::Rename(..) | Operation::Restore(..) => {} + Self::Rename(..) | Self::Restore(..) | Self::CreateDir(..) | Self::RemoveDir(..) => {} } } } @@ -261,6 +276,8 @@ pub(crate) enum Change { Renamed(ProjectPath, ProjectPath), Restored(ProjectPath), Batched(Vec), + DirCreated(ProjectPath), + DirRemoved(ProjectPath), } impl Change { @@ -270,6 +287,8 @@ impl Change { Change::Trashed(worktree_id, trash_id) => Operation::Restore(worktree_id, trash_id), Change::Renamed(from, to) => Operation::Rename(to, from), Change::Restored(project_path) => Operation::Trash(project_path), + Change::DirCreated(path) => Operation::RemoveDir(path), + Change::DirRemoved(path) => Operation::CreateDir(path), // 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: @@ -286,6 +305,31 @@ impl Change { } } +/// Returns the parent directories of `path` that do not yet exist in the given +/// worktree and would therefore be created when an entry is placed at `path`, +/// ordered from the deepest to the shallowest. +pub(crate) fn missing_parent_dirs( + worktree: &Worktree, + worktree_id: WorktreeId, + path: &RelPath, +) -> Vec { + let mut dirs = Vec::new(); + let mut current = path.parent(); + while let Some(parent) = current { + if parent.is_empty() { + break; + } + if worktree.entry_for_path(parent).is_none() { + dirs.push(ProjectPath { + worktree_id, + path: parent.into_arc(), + }); + } + current = parent.parent(); + } + dirs +} + // Imagine pressing undo 10000+ times?! const MAX_UNDO_OPERATIONS: usize = 10_000; @@ -643,6 +687,62 @@ impl Inner { }) } + /// Creates an empty directory at `path`, doing nothing if it already + /// exists. + async fn create_dir(&self, path: &ProjectPath, cx: &mut AsyncApp) -> Result<()> { + let Some(workspace) = self.workspace.upgrade() else { + return Err(anyhow!("Failed to obtain workspace.")); + }; + + let task = workspace.update(cx, |workspace, cx| { + workspace.project().update(cx, |project, cx| { + if project.entry_for_path(path, cx).is_some() { + return None; + } + Some(project.create_entry(path.clone(), true, cx)) + }) + }); + + if let Some(task) = task { + task.await?; + } + + Ok(()) + } + + /// Removes the directory at `path`, but only if it still exists and is + /// empty; otherwise it is left in place. + async fn remove_dir(&self, path: &ProjectPath, cx: &mut AsyncApp) -> Result<()> { + let Some(workspace) = self.workspace.upgrade() else { + return Err(anyhow!("Failed to obtain workspace.")); + }; + + let task = workspace.update(cx, |workspace, cx| { + workspace.project().update(cx, |project, cx| { + let worktree = project.worktree_for_id(path.worktree_id, cx)?; + + let entry_id = { + let worktree = worktree.read(cx); + let entry = worktree.entry_for_path(path.path.as_ref())?; + if !entry.is_dir() + || worktree.child_entries(path.path.as_ref()).next().is_some() + { + return None; + } + entry.id + }; + + project.delete_entry(entry_id, cx) + }) + }); + + if let Some(task) = task { + task.await?; + } + + Ok(()) + } + /// Trashes the specified `project_path` after prompting the user for /// confirmation. If the path has unsaved changes, the prompt also lets the /// user save or discard them.