project_panel: Fix worktree root rename (#24487)

Closes #7923

This PR fixes root worktree renaming by:  

1. Handling the case where `new_path` is the new root name instead of a
relative path from the root.
2. [#20313](https://github.com/zed-industries/zed/pull/20313) added
functionality to watch for root worktree renames made externally, e.g.,
via Finder. This PR avoids relying on that watcher because, when
renaming explicitly from Zed, we can eagerly perform the necessary work
(of course after fs rename) instead of waiting for the watcher to detect
the rename. This prevents UI glitches during renaming root.

Todo:

- [x] Fix wrong abs paths when root is renamed
- [x] Fix explicit scan entry func to handle renamed root dir
- [x] Tests
- [x] Test on Linux
- [x] Tested with single and multipe worktrees
- [x] Tested when single file is root file

Release Notes:

- Fixed an issue where worktree root name couldn't be renamed in project
panel.
This commit is contained in:
smit 2025-02-09 14:16:27 +05:30 committed by GitHub
parent 4207b194e3
commit f1693e6129
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 143 additions and 17 deletions

View file

@ -1535,6 +1535,10 @@ impl Project {
}) })
} }
/// Renames the project entry with given `entry_id`.
///
/// `new_path` is a relative path to worktree root.
/// If root entry is renamed then its new root name is used instead.
pub fn rename_entry( pub fn rename_entry(
&mut self, &mut self,
entry_id: ProjectEntryId, entry_id: ProjectEntryId,
@ -1551,12 +1555,18 @@ impl Project {
}; };
let worktree_id = worktree.read(cx).id(); let worktree_id = worktree.read(cx).id();
let is_root_entry = self.entry_is_worktree_root(entry_id, cx);
let lsp_store = self.lsp_store().downgrade(); let lsp_store = self.lsp_store().downgrade();
cx.spawn(|_, mut cx| async move { cx.spawn(|_, mut cx| async move {
let (old_abs_path, new_abs_path) = { let (old_abs_path, new_abs_path) = {
let root_path = worktree.update(&mut cx, |this, _| this.abs_path())?; let root_path = worktree.update(&mut cx, |this, _| this.abs_path())?;
(root_path.join(&old_path), root_path.join(&new_path)) let new_abs_path = if is_root_entry {
root_path.parent().unwrap().join(&new_path)
} else {
root_path.join(&new_path)
};
(root_path.join(&old_path), new_abs_path)
}; };
LspStore::will_rename_entry( LspStore::will_rename_entry(
lsp_store.clone(), lsp_store.clone(),

View file

@ -733,7 +733,9 @@ impl ProjectPanel {
.action("Copy Path", Box::new(CopyPath)) .action("Copy Path", Box::new(CopyPath))
.action("Copy Relative Path", Box::new(CopyRelativePath)) .action("Copy Relative Path", Box::new(CopyRelativePath))
.separator() .separator()
.action("Rename", Box::new(Rename)) .when(!is_root || !cfg!(target_os = "windows"), |menu| {
menu.action("Rename", Box::new(Rename))
})
.when(!is_root & !is_remote, |menu| { .when(!is_root & !is_remote, |menu| {
menu.action("Trash", Box::new(Trash { skip_prompt: false })) menu.action("Trash", Box::new(Trash { skip_prompt: false }))
}) })
@ -1348,6 +1350,10 @@ impl ProjectPanel {
if let Some(worktree) = self.project.read(cx).worktree_for_id(worktree_id, cx) { if let Some(worktree) = self.project.read(cx).worktree_for_id(worktree_id, cx) {
let sub_entry_id = self.unflatten_entry_id(entry_id); let sub_entry_id = self.unflatten_entry_id(entry_id);
if let Some(entry) = worktree.read(cx).entry_for_id(sub_entry_id) { if let Some(entry) = worktree.read(cx).entry_for_id(sub_entry_id) {
#[cfg(target_os = "windows")]
if Some(entry) == worktree.read(cx).root_entry() {
return;
}
self.edit_state = Some(EditState { self.edit_state = Some(EditState {
worktree_id, worktree_id,
entry_id: sub_entry_id, entry_id: sub_entry_id,
@ -7280,6 +7286,84 @@ mod tests {
); );
} }
#[gpui::test]
#[cfg_attr(target_os = "windows", ignore)]
async fn test_rename_root_of_worktree(cx: &mut gpui::TestAppContext) {
init_test_with_editor(cx);
let fs = FakeFs::new(cx.executor().clone());
fs.insert_tree(
"/root1",
json!({
"dir1": {
"file1.txt": "content 1",
},
}),
)
.await;
let project = Project::test(fs.clone(), ["/root1".as_ref()], cx).await;
let workspace =
cx.add_window(|window, cx| Workspace::test_new(project.clone(), window, cx));
let cx = &mut VisualTestContext::from_window(*workspace, cx);
let panel = workspace.update(cx, ProjectPanel::new).unwrap();
toggle_expand_dir(&panel, "root1/dir1", cx);
assert_eq!(
visible_entries_as_strings(&panel, 0..20, cx),
&["v root1", " v dir1 <== selected", " file1.txt",],
"Initial state with worktrees"
);
select_path(&panel, "root1", cx);
assert_eq!(
visible_entries_as_strings(&panel, 0..20, cx),
&["v root1 <== selected", " v dir1", " file1.txt",],
);
// Rename root1 to new_root1
panel.update_in(cx, |panel, window, cx| panel.rename(&Rename, window, cx));
assert_eq!(
visible_entries_as_strings(&panel, 0..20, cx),
&[
"v [EDITOR: 'root1'] <== selected",
" v dir1",
" file1.txt",
],
);
let confirm = panel.update_in(cx, |panel, window, cx| {
panel
.filename_editor
.update(cx, |editor, cx| editor.set_text("new_root1", window, cx));
panel.confirm_edit(window, cx).unwrap()
});
confirm.await.unwrap();
assert_eq!(
visible_entries_as_strings(&panel, 0..20, cx),
&[
"v new_root1 <== selected",
" v dir1",
" file1.txt",
],
"Should update worktree name"
);
// Ensure internal paths have been updated
select_path(&panel, "new_root1/dir1/file1.txt", cx);
assert_eq!(
visible_entries_as_strings(&panel, 0..20, cx),
&[
"v new_root1",
" v dir1",
" file1.txt <== selected",
],
"Files in renamed worktree are selectable"
);
}
#[gpui::test] #[gpui::test]
async fn test_multiple_marked_entries(cx: &mut gpui::TestAppContext) { async fn test_multiple_marked_entries(cx: &mut gpui::TestAppContext) {
init_test_with_editor(cx); init_test_with_editor(cx);

View file

@ -1432,16 +1432,7 @@ impl LocalWorktree {
drop(barrier); drop(barrier);
} }
ScanState::RootUpdated { new_path } => { ScanState::RootUpdated { new_path } => {
if let Some(new_path) = new_path { this.update_abs_path_and_refresh(new_path, cx);
this.snapshot.git_repositories = Default::default();
this.snapshot.ignores_by_parent_abs_path = Default::default();
let root_name = new_path
.as_path()
.file_name()
.map_or(String::new(), |f| f.to_string_lossy().to_string());
this.snapshot.update_abs_path(new_path, root_name);
}
this.restart_background_scanners(cx);
} }
} }
cx.notify(); cx.notify();
@ -1881,6 +1872,10 @@ impl LocalWorktree {
})) }))
} }
/// Rename an entry.
///
/// `new_path` is the new relative path to the worktree root.
/// If the root entry is renamed then `new_path` is the new root name instead.
fn rename_entry( fn rename_entry(
&self, &self,
entry_id: ProjectEntryId, entry_id: ProjectEntryId,
@ -1893,8 +1888,18 @@ impl LocalWorktree {
}; };
let new_path = new_path.into(); let new_path = new_path.into();
let abs_old_path = self.absolutize(&old_path); let abs_old_path = self.absolutize(&old_path);
let Ok(abs_new_path) = self.absolutize(&new_path) else {
return Task::ready(Err(anyhow!("absolutizing path {new_path:?}"))); let is_root_entry = self.root_entry().is_some_and(|e| e.id == entry_id);
let abs_new_path = if is_root_entry {
let Some(root_parent_path) = self.abs_path().parent() else {
return Task::ready(Err(anyhow!("no parent for path {:?}", self.abs_path)));
};
root_parent_path.join(&new_path)
} else {
let Ok(absolutize_path) = self.absolutize(&new_path) else {
return Task::ready(Err(anyhow!("absolutizing path {new_path:?}")));
};
absolutize_path
}; };
let abs_path = abs_new_path.clone(); let abs_path = abs_new_path.clone();
let fs = self.fs.clone(); let fs = self.fs.clone();
@ -1928,9 +1933,19 @@ impl LocalWorktree {
rename.await?; rename.await?;
Ok(this Ok(this
.update(&mut cx, |this, cx| { .update(&mut cx, |this, cx| {
this.as_local_mut() let local = this.as_local_mut().unwrap();
.unwrap() if is_root_entry {
.refresh_entry(new_path.clone(), Some(old_path), cx) // We eagerly update `abs_path` and refresh this worktree.
// Otherwise, the FS watcher would do it on the `RootUpdated` event,
// but with a noticeable delay, so we handle it proactively.
local.update_abs_path_and_refresh(
Some(SanitizedPath::from(abs_path.clone())),
cx,
);
Task::ready(Ok(this.root_entry().cloned()))
} else {
local.refresh_entry(new_path.clone(), Some(old_path), cx)
}
})? })?
.await? .await?
.map(CreatedEntry::Included) .map(CreatedEntry::Included)
@ -2195,6 +2210,23 @@ impl LocalWorktree {
self.share_private_files = true; self.share_private_files = true;
self.restart_background_scanners(cx); self.restart_background_scanners(cx);
} }
fn update_abs_path_and_refresh(
&mut self,
new_path: Option<SanitizedPath>,
cx: &Context<Worktree>,
) {
if let Some(new_path) = new_path {
self.snapshot.git_repositories = Default::default();
self.snapshot.ignores_by_parent_abs_path = Default::default();
let root_name = new_path
.as_path()
.file_name()
.map_or(String::new(), |f| f.to_string_lossy().to_string());
self.snapshot.update_abs_path(new_path, root_name);
}
self.restart_background_scanners(cx);
}
} }
impl RemoteWorktree { impl RemoteWorktree {