git_ui: Resolve worktree picker remove-from-window clicks by path (#59084)

Follow-up to #58996. The "Remove Worktree from Window" button captured
the row index at render time and re-resolved it against the picker's
matches at click time. Matches are rebuilt asynchronously (initial
worktree list load, post-delete refresh, query changes), so the index
could drift between the rendered frame and the click being processed,
closing the workspace of a different worktree than the one the user
clicked.

This changes the click handler to capture the worktree's path — a stable
identity — instead of its index, adds the same `deleting_worktree_paths`
click-time guard the neighboring handlers use, and constructs the button
only when it will actually be shown.

Scoped to the code added in #58996; the same stale-index pattern in the
delete and open-in-new-window handlers is left for a later change.

Release Notes:

- Fixed an issue where the worktree picker's "Remove Worktree from
Window" button could act on the wrong worktree if the list updated at
the moment of the click.
This commit is contained in:
MartinYe1234 authored and GitHub committed 2026-06-16 02:32:29 +00:00
1 parent 21c176e13a
commit 7e6b9e0cff
1 file changed
+23 -18
+23 -18
View File
@@ -702,15 +702,14 @@ impl WorktreePickerDelegate {
fn remove_worktree_from_window(
&mut self,
ix: usize,
worktree_path: &Path,
window: &mut Window,
cx: &mut Context<Picker<Self>>,
) {
let Some(WorktreeEntry::Worktree { worktree, .. }) = self.matches.get(ix) else {
if self.deleting_worktree_paths.contains(worktree_path) {
return;
};
let Some(workspace_to_remove) =
self.workspace_for_open_worktree(&worktree.path, window, cx)
}
let Some(workspace_to_remove) = self.workspace_for_open_worktree(worktree_path, window, cx)
else {
return;
};
@@ -1314,22 +1313,29 @@ impl PickerDelegate for WorktreePickerDelegate {
})),
);
let remove_from_window_button = IconButton::new(
("remove-worktree-from-window", ix),
IconName::Close,
)
.icon_size(IconSize::Small)
.tooltip(Tooltip::text("Remove Worktree from Window"))
.on_click(cx.listener(move |picker, _, window, cx| {
picker.delegate.remove_worktree_from_window(ix, window, cx);
}));
this.end_slot(
h_flex()
.gap_0p5()
.child(open_in_new_window_button)
.when(can_remove_from_window, |this| {
this.child(remove_from_window_button)
let worktree_path = worktree.path.clone();
this.child(
IconButton::new(
("remove-worktree-from-window", ix),
IconName::Close,
)
.icon_size(IconSize::Small)
.tooltip(Tooltip::text("Remove Worktree from Window"))
.on_click(
cx.listener(move |picker, _, window, cx| {
picker.delegate.remove_worktree_from_window(
&worktree_path,
window,
cx,
);
}),
),
)
})
.when(can_delete, |this| this.child(delete_button)),
)
@@ -2211,7 +2217,6 @@ mod tests {
});
cx.run_until_parked();
let index = worktree_index(&worktree_picker, &worktree_path, &mut cx);
worktree_picker.update(&mut cx, |worktree_picker, cx| {
worktree_picker.picker.update(cx, |picker, _| {
assert!(
@@ -2228,7 +2233,7 @@ mod tests {
worktree_picker.picker.update(cx, |picker, cx| {
picker
.delegate
.remove_worktree_from_window(index, window, cx);
.remove_worktree_from_window(&worktree_path, window, cx);
})
});
cx.run_until_parked();