project_panel: Remove directories created by rename on undo (#60082)

# Objective

When an user uses the Project Panel to rename a file, it is possible to
change its path too. For example, renaming `README.md` to
`documents/README.md` will create the `documents/` directory, in case it
doesn't yet exist.

Unfortunately, even though undoing a rename operation is already
supported, we were not yet considering this scenario where directories
had been created specifically to support the rename. As such, when undo
was used, for the scenario above, we'd end up moving `README.md` back to
its original location but would leave the empty `documents/` directory
behind.

## Solution

Introduce two new operations and changes specifically for this use-case,
`Operation::CreateDir`, `Operation::RemoveDir`, `Change::DirCreated` and
`Change::DirRemoved`, which we can then batch together with the
`Operation::Rename` in case directories need to be created or removed.

An initial approach of just keeping the list of created directories in
the `Operation::Rename` and `Change::Renamed` variants was considered
but it would require all users of `Operation::Rename` to now set it,
even if they don't actually need it, like the drag and paste operations.
Having separate operations and change variants also makes it clearer
what these are meant to be used for.

Something else worth noting is that, for `Operation::CreateDir`, the
directory will only be created if it doesn't yet exist, otherwise we'll
ignore. Same happens for `Operation::RemoveDir`, where if the directory
is not empty, we don't delete it, as it's possible for new files to have
been added to the directory outside of Zed and we don't want to delete
those.

## Testing

Tested both manually as well as introduced a new test case
– `project_panel::tests::undo::rename_with_dir_undo_redo` .

## Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [ ] Unsafe blocks (if any) have justifying comments
- [ ] The content adheres to Zed's UI standards
([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
and
[icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md)
guidelines)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

## Showcase

> This section is optional. If this PR does not include a visual change
or does not add a new user-facing feature, you can delete this section.

- Help others understand the result of this PR by showcasing your
awesome work!
- If this PR includes a visual change, consider adding a screenshot,
GIF, or video
- A before/after comparison is very useful for changes to existing
features!

While a showcase should aim to be brief and digestible, you can use a
toggleable section to save space on longer showcases:

<details>
  <summary>Before</summary>


https://github.com/user-attachments/assets/15384fa8-9b94-495c-8992-19eee7a8f038
</details>

<details>
  <summary>After</summary>


https://github.com/user-attachments/assets/c5be9f38-a2b4-4acc-8a1e-178161e326bb
</details>

---

Release Notes:

- Fixed undoing a file rename leaving behind directories created by the
rename.
This commit is contained in:
Dino authored and GitHub committed 2026-08-20 11:25:47 +00:00
1 parent 629da95e29
commit d7fb2dfe74
3 files changed
+183 -17

No files matched your search

+26 -12
View File
@@ -1937,8 +1937,9 @@ impl ProjectPanel {
let edit_task;
let edited_entry_id;
let edited_entry;
let new_project_path: ProjectPath;
let changes: Vec<Change>;
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
+53 -1
View File
@@ -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;
+104 -4
View File
@@ -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<Operation>),
/// 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<Change>),
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<ProjectPath> {
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.