Show progress when deleting worktrees (#57751)

Adds a progress indicator to the worktree picker so users get visual
feedback while a worktree deletion is in progress, which can take
several seconds.

Closes AI-239

Release Notes:

- Added progress feedback in the worktree picker while deleting a
worktree
This commit is contained in:
MartinYe1234 2026-05-28 14:52:53 -07:00 committed by GitHub
parent 27c566c212
commit 59d8766f35
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -14,8 +14,8 @@ use picker::{Picker, PickerDelegate, PickerEditorPosition};
use project::Project;
use project::git_store::RepositoryEvent;
use ui::{
Button, Divider, HighlightedLabel, IconButton, KeyBinding, ListItem, ListItemSpacing, Tooltip,
prelude::*,
Button, CommonAnimationExt as _, Divider, HighlightedLabel, IconButton, KeyBinding, ListItem,
ListItemSpacing, Tooltip, prelude::*,
};
use util::ResultExt as _;
use util::paths::PathExt;
@ -116,6 +116,7 @@ impl WorktreePicker {
show_footer,
modifiers: Modifiers::default(),
hovered_delete_index: None,
deleting_worktree_paths: HashSet::default(),
};
let picker = cx.new(|cx| {
@ -313,6 +314,7 @@ struct WorktreePickerDelegate {
show_footer: bool,
modifiers: Modifiers,
hovered_delete_index: Option<usize>,
deleting_worktree_paths: HashSet<PathBuf>,
}
fn remove_worktree_command(path: &Path, force: bool) -> String {
@ -464,7 +466,7 @@ impl WorktreePickerDelegate {
}
fn delete_worktree(
&self,
&mut self,
ix: usize,
force: bool,
window: &mut Window,
@ -476,7 +478,9 @@ impl WorktreePickerDelegate {
let WorktreeEntry::Worktree { worktree, .. } = entry else {
return;
};
if !self.can_delete_worktree(worktree) {
if !self.can_delete_worktree(worktree)
|| self.deleting_worktree_paths.contains(&worktree.path)
{
return;
}
@ -493,10 +497,27 @@ impl WorktreePickerDelegate {
);
let workspace = self.workspace.clone();
self.deleting_worktree_paths.insert(path.clone());
if self.hovered_delete_index == Some(ix) {
self.hovered_delete_index = None;
}
cx.notify();
cx.spawn_in(window, async move |picker, cx| {
let initial_result = repo
let initial_result = match repo
.update(cx, |repo, _| repo.remove_worktree(path.clone(), force))
.await?;
.await
{
Ok(result) => result,
Err(error) => {
picker.update_in(cx, |picker, _window, cx| {
if picker.delegate.deleting_worktree_paths.remove(&path) {
cx.notify();
}
})?;
return Err(error.into());
}
};
let (result, attempted_force) = match initial_result {
Ok(()) => (Ok(()), force),
@ -510,6 +531,12 @@ impl WorktreePickerDelegate {
.flatten();
if let Some(prompt_message) = force_delete_prompt {
picker.update_in(cx, |picker, _window, cx| {
if picker.delegate.deleting_worktree_paths.remove(&path) {
cx.notify();
}
})?;
let answer = cx.update(|window, cx| {
window.prompt(
PromptLevel::Warning,
@ -524,9 +551,39 @@ impl WorktreePickerDelegate {
return Ok(());
}
let retry = repo
let should_retry = picker.update_in(cx, |picker, _window, cx| {
let worktree_still_exists = picker
.delegate
.all_worktrees
.iter()
.any(|worktree| worktree.path == path);
if !worktree_still_exists
|| !picker.delegate.deleting_worktree_paths.insert(path.clone())
{
return false;
}
cx.notify();
true
})?;
if !should_retry {
return Ok(());
}
let retry = match repo
.update(cx, |repo, _| repo.remove_worktree(path.clone(), true))
.await?;
.await
{
Ok(result) => result,
Err(error) => {
picker.update_in(cx, |picker, _window, cx| {
if picker.delegate.deleting_worktree_paths.remove(&path) {
cx.notify();
}
})?;
return Err(error.into());
}
};
if let Err(error) = &retry {
log::error!("Failed to force remove worktree: {error}");
@ -540,6 +597,12 @@ impl WorktreePickerDelegate {
};
if let Err(error) = result {
picker.update_in(cx, |picker, _window, cx| {
if picker.delegate.deleting_worktree_paths.remove(&path) {
cx.notify();
}
})?;
if let Some(workspace) = workspace.upgrade() {
cx.update(|_window, cx| {
show_error_toast(
@ -555,6 +618,7 @@ impl WorktreePickerDelegate {
}
picker.update_in(cx, |picker, _window, cx| {
picker.delegate.deleting_worktree_paths.remove(&path);
picker.delegate.matches.retain(|e| {
!matches!(e, WorktreeEntry::Worktree { worktree, .. } if worktree.path == path)
});
@ -814,6 +878,10 @@ impl PickerDelegate for WorktreePickerDelegate {
}
}
WorktreeEntry::Worktree { worktree, .. } => {
if self.deleting_worktree_paths.contains(&worktree.path) {
return;
}
let is_current = self.project_worktree_paths.contains(&worktree.path);
if !is_current {
@ -956,6 +1024,7 @@ impl PickerDelegate for WorktreePickerDelegate {
let sha = worktree.sha.chars().take(7).collect::<String>();
let is_current = self.project_worktree_paths.contains(&worktree.path);
let is_deleting = self.deleting_worktree_paths.contains(&worktree.path);
let can_delete = self.can_delete_worktree(worktree);
let entry_icon = if is_current {
@ -1035,7 +1104,24 @@ impl PickerDelegate for WorktreePickerDelegate {
),
),
)
.when(!is_current, |this| {
.when(is_deleting, |this| {
this.end_slot(
h_flex()
.gap_1()
.child(
Icon::new(IconName::LoadCircle)
.size(IconSize::Small)
.color(Color::Muted)
.with_rotate_animation(2),
)
.child(
Label::new("Deleting…")
.size(LabelSize::Small)
.color(Color::Muted),
),
)
})
.when(!is_deleting && !is_current, |this| {
let open_in_new_window_button =
IconButton::new(("open-new-window", ix), IconName::ArrowUpRight)
.icon_size(IconSize::Small)
@ -1045,6 +1131,13 @@ impl PickerDelegate for WorktreePickerDelegate {
return;
};
if let WorktreeEntry::Worktree { worktree, .. } = entry {
if picker
.delegate
.deleting_worktree_paths
.contains(&worktree.path)
{
return;
}
window.dispatch_action(
Box::new(OpenWorktreeInNewWindow {
path: worktree.path.clone(),
@ -1083,12 +1176,8 @@ impl PickerDelegate for WorktreePickerDelegate {
.into()
})
.on_click(cx.listener(move |picker, _, window, cx| {
picker.delegate.delete_worktree(
ix,
picker.delegate.modifiers.alt,
window,
cx,
);
let force = picker.delegate.modifiers.alt;
picker.delegate.delete_worktree(ix, force, window, cx);
})),
);
@ -1162,6 +1251,10 @@ impl PickerDelegate for WorktreePickerDelegate {
matches!(e, WorktreeEntry::Worktree { worktree, .. } if self.project_worktree_paths.contains(&worktree.path))
});
let is_deleting = selected_entry.is_some_and(|e| {
matches!(e, WorktreeEntry::Worktree { worktree, .. } if self.deleting_worktree_paths.contains(&worktree.path))
});
let footer = h_flex()
.w_full()
.p_1p5()
@ -1188,7 +1281,14 @@ impl PickerDelegate for WorktreePickerDelegate {
} else if is_existing_worktree {
Some(
footer
.when(can_delete, |this| {
.when(is_deleting, |this| {
this.child(
Button::new("delete-worktree", "Deleting…")
.loading(true)
.disabled(true),
)
})
.when(!is_deleting && can_delete, |this| {
let focus_handle = focus_handle.clone();
this.child(
Button::new("delete-worktree", "Delete")
@ -1201,7 +1301,7 @@ impl PickerDelegate for WorktreePickerDelegate {
}),
)
})
.when(!is_current, |this| {
.when(!is_deleting && !is_current, |this| {
let focus_handle = focus_handle.clone();
this.child(
Button::new("open-in-new-window", "Open in New Window")
@ -1218,7 +1318,8 @@ impl PickerDelegate for WorktreePickerDelegate {
}),
)
})
.child(
.when(!is_deleting, |this| {
this.child(
Button::new("open-worktree", "Open")
.key_binding(
KeyBinding::for_action_in(&menu::Confirm, &focus_handle, cx)
@ -1228,6 +1329,7 @@ impl PickerDelegate for WorktreePickerDelegate {
window.dispatch_action(menu::Confirm.boxed_clone(), cx)
}),
)
})
.into_any(),
)
} else {
@ -1482,6 +1584,33 @@ mod tests {
})
}
fn picker_contains_worktree(
worktree_picker: &Entity<WorktreePicker>,
worktree_path: &Path,
cx: &mut VisualTestContext,
) -> bool {
worktree_picker.update(cx, |worktree_picker, cx| {
worktree_picker.picker.update(cx, |picker, _| {
picker.delegate.all_worktrees.iter().any(|worktree| {
worktree.path == *worktree_path
}) && picker.delegate.matches.iter().any(|entry| {
matches!(entry, WorktreeEntry::Worktree { worktree, .. } if worktree.path == *worktree_path)
})
})
})
}
fn deleting_worktree_paths(
worktree_picker: &Entity<WorktreePicker>,
cx: &mut VisualTestContext,
) -> HashSet<PathBuf> {
worktree_picker.update(cx, |worktree_picker, cx| {
worktree_picker.picker.update(cx, |picker, _| {
picker.delegate.deleting_worktree_paths.clone()
})
})
}
async fn repo_contains_worktree(
repository: &Entity<project::git_store::Repository>,
worktree_path: &Path,
@ -1497,6 +1626,54 @@ mod tests {
.any(|worktree| worktree.path == *worktree_path)
}
#[gpui::test]
async fn test_delete_worktree_marks_row_pending_immediately(cx: &mut TestAppContext) {
let (_, worktree_picker, _repository, worktree_path, mut cx) =
init_worktree_picker_test(cx).await;
let index = worktree_index(&worktree_picker, &worktree_path, &mut cx);
worktree_picker.update_in(&mut cx, |worktree_picker, window, cx| {
worktree_picker.picker.update(cx, |picker, cx| {
picker.delegate.delete_worktree(index, false, window, cx);
})
});
let pending_paths = deleting_worktree_paths(&worktree_picker, &mut cx);
assert_eq!(pending_paths.len(), 1);
assert!(pending_paths.contains(&worktree_path));
cx.run_until_parked();
}
#[gpui::test]
async fn test_delete_worktree_clears_pending_and_removes_row_on_success(
cx: &mut TestAppContext,
) {
let (_, worktree_picker, repository, worktree_path, mut cx) =
init_worktree_picker_test(cx).await;
let index = worktree_index(&worktree_picker, &worktree_path, &mut cx);
worktree_picker.update_in(&mut cx, |worktree_picker, window, cx| {
worktree_picker.picker.update(cx, |picker, cx| {
picker.delegate.delete_worktree(index, false, window, cx);
})
});
assert!(deleting_worktree_paths(&worktree_picker, &mut cx).contains(&worktree_path));
cx.run_until_parked();
assert!(deleting_worktree_paths(&worktree_picker, &mut cx).is_empty());
assert!(!picker_contains_worktree(
&worktree_picker,
&worktree_path,
&mut cx
));
assert!(
!repo_contains_worktree(&repository, &worktree_path, &mut cx).await,
"worktree should be removed after successful delete"
);
}
#[gpui::test]
async fn test_remote_default_branch_is_preferred_create_target(cx: &mut TestAppContext) {
let (_fs, worktree_picker, _repository, _worktree_path, mut cx) =
@ -1588,19 +1765,96 @@ mod tests {
picker.delegate.delete_worktree(index, false, window, cx);
})
});
assert!(deleting_worktree_paths(&worktree_picker, &mut cx).contains(&worktree_path));
cx.run_until_parked();
assert!(cx.has_pending_prompt());
assert!(
!deleting_worktree_paths(&worktree_picker, &mut cx).contains(&worktree_path),
"pending delete state should clear while waiting for force-delete confirmation"
);
cx.simulate_prompt_answer("Force Delete");
cx.run_until_parked();
assert!(!cx.has_pending_prompt());
assert!(deleting_worktree_paths(&worktree_picker, &mut cx).is_empty());
assert!(!picker_contains_worktree(
&worktree_picker,
&worktree_path,
&mut cx
));
assert!(
!repo_contains_worktree(&repository, &worktree_path, &mut cx).await,
"worktree should be removed after confirming force delete"
);
}
#[gpui::test]
async fn test_duplicate_delete_worktree_is_ignored_while_pending(cx: &mut TestAppContext) {
let (fs, worktree_picker, _repository, worktree_path, mut cx) =
init_worktree_picker_test(cx).await;
fs.with_git_state(path!("/root/project/.git").as_ref(), true, |state| {
state
.worktrees_requiring_force_delete
.insert(worktree_path.clone());
})
.expect("failed to mark test worktree as requiring force delete");
let index = worktree_index(&worktree_picker, &worktree_path, &mut cx);
worktree_picker.update_in(&mut cx, |worktree_picker, window, cx| {
worktree_picker.picker.update(cx, |picker, cx| {
picker.delegate.delete_worktree(index, false, window, cx);
picker.delegate.delete_worktree(index, false, window, cx);
})
});
let pending_paths = deleting_worktree_paths(&worktree_picker, &mut cx);
assert_eq!(pending_paths.len(), 1);
assert!(pending_paths.contains(&worktree_path));
cx.run_until_parked();
assert!(cx.has_pending_prompt());
assert!(deleting_worktree_paths(&worktree_picker, &mut cx).is_empty());
cx.simulate_prompt_answer("Cancel");
cx.run_until_parked();
assert!(!cx.has_pending_prompt());
assert!(picker_contains_worktree(
&worktree_picker,
&worktree_path,
&mut cx
));
}
#[gpui::test]
async fn test_selected_deleting_worktree_cannot_be_opened(cx: &mut TestAppContext) {
let (_, worktree_picker, _repository, worktree_path, mut cx) =
init_worktree_picker_test(cx).await;
let subscription = cx.update(|_, cx| {
cx.subscribe(&worktree_picker, |_, _: &DismissEvent, _| {
panic!("DismissEvent should not be emitted for a deleting worktree");
})
});
let index = worktree_index(&worktree_picker, &worktree_path, &mut cx);
worktree_picker.update_in(&mut cx, |worktree_picker, window, cx| {
worktree_picker.picker.update(cx, |picker, cx| {
picker.delegate.selected_index = index;
picker.delegate.delete_worktree(index, false, window, cx);
picker.delegate.confirm(false, window, cx);
})
});
assert!(deleting_worktree_paths(&worktree_picker, &mut cx).contains(&worktree_path));
drop(subscription);
cx.run_until_parked();
}
#[gpui::test]
async fn test_force_delete_worktree_deletes_without_prompt(cx: &mut TestAppContext) {
let (fs, worktree_picker, repository, worktree_path, mut cx) =
@ -1620,9 +1874,17 @@ mod tests {
picker.delegate.delete_worktree(index, true, window, cx);
})
});
assert!(deleting_worktree_paths(&worktree_picker, &mut cx).contains(&worktree_path));
cx.run_until_parked();
assert!(!cx.has_pending_prompt());
assert!(deleting_worktree_paths(&worktree_picker, &mut cx).is_empty());
assert!(!picker_contains_worktree(
&worktree_picker,
&worktree_path,
&mut cx
));
assert!(
!repo_contains_worktree(&repository, &worktree_path, &mut cx).await,
"worktree should be removed by explicit force delete"