Skip to content

Commit 2f06150

Browse files
Copilotalexr00
andauthored
Replace branch deletion picker with a modal dialog (#8947)
* Initial plan * Replace branch deletion picker with concise modal actions Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com> * Better wording * Fix test * Fix another test --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>
1 parent c88a2f5 commit 2f06150

7 files changed

Lines changed: 294 additions & 51 deletions

File tree

package.nls.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,9 @@
4343
"githubPullRequests.fileListLayout.description": "The layout to use when displaying changed files list.",
4444
"githubPullRequests.hideViewedFiles.description": "Hide files that have been marked as viewed in the pull request changes tree.",
4545
"githubPullRequests.fileAutoReveal.description": "Automatically reveal open files in the pull request changes tree.",
46-
"githubPullRequests.defaultDeletionMethod.selectLocalBranch.description": "When true, the option to delete the local branch will be selected by default when deleting a branch from a pull request.",
47-
"githubPullRequests.defaultDeletionMethod.selectRemote.description": "When true, the option to delete the remote will be selected by default when deleting a branch from a pull request.",
48-
"githubPullRequests.defaultDeletionMethod.selectWorktree.description": "When true, the option to remove the associated worktree will be selected by default when deleting a branch from a pull request.",
46+
"githubPullRequests.defaultDeletionMethod.selectLocalBranch.description": "When true, delete the local branch during automatic branch cleanup after merging a pull request or adding it to a merge queue.",
47+
"githubPullRequests.defaultDeletionMethod.selectRemote.description": "When true, delete the unused remote during automatic branch cleanup after merging a pull request.",
48+
"githubPullRequests.defaultDeletionMethod.selectWorktree.description": "When true, remove the associated worktree during automatic branch cleanup after merging a pull request, and select worktree removal by default during bulk branch cleanup.",
4949
"githubPullRequests.deleteBranchAfterMerge.description": "Automatically delete the branch after merging a pull request. This setting only applies when the pull request is merged through this extension. When using merge queues, this will only delete the local branch.",
5050
"githubPullRequests.enableAttestationCommits.description": "Enables adding an attestation commit (an empty, signed commit) to the head of a pull request branch as a way to attest to a pull request even when its individual commits are unsigned. Requires commit signing to be configured for git. Set to `true` to enable with the default commit message, set to a string to use that string as the commit message, or set to `false` to disable.",
5151
"githubPullRequests.terminalLinksHandler.description": "Default handler for terminal links.",

src/github/activityBarViewProvider.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -447,11 +447,7 @@ export class PullRequestViewProvider extends WebviewViewBase implements vscode.W
447447

448448
private async deleteBranch(message: IRequestMessage<any>) {
449449
const result = await PullRequestReviewCommon.deleteBranch(this._folderRepositoryManager, this._item);
450-
if (result.isReply) {
451-
this._replyMessage(message, result.message);
452-
} else {
453-
this._postMessage(result.message);
454-
}
450+
await this._replyMessage(message, result.message);
455451
}
456452

457453
private async setReadyForReview(message: IRequestMessage<Record<string, unknown>>): Promise<void> {

src/github/pullRequestOverview.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,12 +1046,10 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
10461046

10471047
private async deleteBranch(message: IRequestMessage<any>) {
10481048
const result = await PullRequestReviewCommon.deleteBranch(this._folderRepositoryManager, this._item);
1049-
if (result.isReply) {
1050-
this._replyMessage(message, result.message);
1051-
} else {
1049+
if (!result.isReply) {
10521050
this.refreshPanel();
1053-
this._postMessage(result.message);
10541051
}
1052+
await this._replyMessage(message, result.message);
10551053
}
10561054

10571055
private async setReadyForReview(message: IRequestMessage<{}>): Promise<void> {

src/github/pullRequestReviewCommon.ts

Lines changed: 53 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -306,9 +306,15 @@ export namespace PullRequestReviewCommon {
306306
});
307307
}
308308

309+
function isBranchNotFoundError(error: unknown): boolean {
310+
const stderr = error && typeof error === 'object' ? Reflect.get(error, 'stderr') : undefined;
311+
return typeof stderr === 'string' && stderr.includes('not found');
312+
}
313+
309314
export async function deleteBranch(folderRepositoryManager: FolderRepositoryManager, item: PullRequestModel): Promise<{ isReply: boolean, message: any }> {
310315
const branchInfo = await folderRepositoryManager.getBranchNameForPullRequest(item);
311-
const actions: (vscode.QuickPickItem & SelectedAction)[] = [];
316+
const actions: (vscode.MessageItem & SelectedAction)[] = [];
317+
const cleanupDetails: string[] = [];
312318
const defaultBranch = await folderRepositoryManager.getPullRequestRepositoryDefaultBranch(item);
313319

314320
if (item.isResolved()) {
@@ -317,56 +323,52 @@ export namespace PullRequestReviewCommon {
317323

318324
const isDefaultBranch = defaultBranch === item.head.ref;
319325
if (!isDefaultBranch && !item.isRemoteHeadDeleted) {
326+
const remoteBranch = headRepo ? `${headRepo.remote.remoteName}/${branchHeadRef}` : branchHeadRef;
327+
const remoteRepository = item.head.repositoryCloneUrl.toString() ??
328+
`${item.remote.normalizedHost}/${item.head.repositoryCloneUrl.owner}/${item.head.repositoryCloneUrl.repositoryName}`;
320329
actions.push({
321-
label: vscode.l10n.t('Delete remote branch {0}', `${headRepo?.remote.remoteName}/${branchHeadRef}`),
322-
description: `${item.remote.normalizedHost}/${item.head.repositoryCloneUrl.owner}/${item.remote.repositoryName}`,
330+
title: vscode.l10n.t('Delete Remote Branch'),
323331
type: 'remoteHead',
324-
picked: true,
325332
});
333+
cleanupDetails.push(
334+
vscode.l10n.t('Remote branch: {0}', remoteBranch),
335+
vscode.l10n.t('Remote repository: {0}', remoteRepository),
336+
);
326337
}
327338
}
328339

329340
if (branchInfo) {
330-
const preferredLocalBranchDeletionMethod = vscode.workspace
331-
.getConfiguration(PR_SETTINGS_NAMESPACE)
332-
.get<boolean>(`${DEFAULT_DELETION_METHOD}.${SELECT_LOCAL_BRANCH}`);
333341
actions.push({
334-
label: vscode.l10n.t('Delete local branch {0}', branchInfo.branch),
342+
title: vscode.l10n.t('Delete Local Branch'),
335343
type: 'local',
336-
picked: !!preferredLocalBranchDeletionMethod,
337344
});
338-
339-
const preferredRemoteDeletionMethod = vscode.workspace
340-
.getConfiguration(PR_SETTINGS_NAMESPACE)
341-
.get<boolean>(`${DEFAULT_DELETION_METHOD}.${SELECT_REMOTE}`);
345+
cleanupDetails.push(vscode.l10n.t('Local branch: {0}', branchInfo.branch));
342346

343347
if (branchInfo.remote && branchInfo.createdForPullRequest && !branchInfo.remoteInUse) {
344348
actions.push({
345-
label: vscode.l10n.t('Delete remote {0}, which is no longer used by any other branch', branchInfo.remote),
349+
title: vscode.l10n.t('Delete Remote'),
346350
type: 'remote',
347-
picked: !!preferredRemoteDeletionMethod,
348351
});
352+
cleanupDetails.push(vscode.l10n.t('Unused Git remote: {0}', branchInfo.remote));
349353
}
350354

351355
const worktreePath = folderRepositoryManager.getWorktreeForBranch(branchInfo.branch);
352356
if (worktreePath && !isWorktreeInWorkspace(worktreePath)) {
353-
const preferredWorktreeDeletion = vscode.workspace
354-
.getConfiguration(PR_SETTINGS_NAMESPACE)
355-
.get<boolean>(`${DEFAULT_DELETION_METHOD}.${SELECT_WORKTREE}`);
356357
actions.push({
357-
label: vscode.l10n.t('Remove worktree {0}', worktreePath.fsPath),
358+
title: vscode.l10n.t('Remove Worktree'),
358359
type: 'worktree',
359360
worktreePath: worktreePath.fsPath,
360-
picked: !!preferredWorktreeDeletion,
361361
});
362+
cleanupDetails.push(vscode.l10n.t('Worktree: {0}', worktreePath.fsPath));
362363
}
363364
}
364365

365366
if (vscode.env.remoteName === 'codespaces') {
366367
actions.push({
367-
label: vscode.l10n.t('Suspend Codespace'),
368+
title: vscode.l10n.t('Suspend Codespace'),
368369
type: 'suspend'
369370
});
371+
cleanupDetails.push(vscode.l10n.t('Codespace: current Codespace'));
370372
}
371373

372374
if (!actions.length) {
@@ -381,14 +383,28 @@ export namespace PullRequestReviewCommon {
381383
};
382384
}
383385

384-
const selectedActions = await vscode.window.showQuickPick(actions, {
385-
canPickMany: true,
386-
ignoreFocusOut: true,
387-
});
388-
389-
390-
if (selectedActions) {
391-
const deletedBranchTypes: string[] = await performBranchDeletion(folderRepositoryManager, item, defaultBranch, branchInfo!, selectedActions);
386+
const options: (vscode.MessageItem & { actions: SelectedAction[] })[] = actions.map(action => ({
387+
title: action.title,
388+
actions: [action],
389+
}));
390+
const deletionActions = actions.filter(action => action.type !== 'suspend');
391+
if (deletionActions.length > 1) {
392+
options.unshift({ title: vscode.l10n.t('Delete All'), actions: deletionActions });
393+
}
394+
const selectedOption = await vscode.window.showWarningMessage(
395+
vscode.l10n.t('Choose what to delete for Pull Request #{0}', item.number),
396+
{
397+
modal: true,
398+
detail: vscode.l10n.t(
399+
'Choose an action below to clean up the resources associated with this pull request.\n\n{0}',
400+
cleanupDetails.join('\n'),
401+
)
402+
},
403+
...options,
404+
);
405+
406+
if (selectedOption) {
407+
const deletedBranchTypes: string[] = await performBranchDeletion(folderRepositoryManager, item, defaultBranch, branchInfo!, selectedOption.actions);
392408

393409
return {
394410
isReply: false,
@@ -462,7 +478,14 @@ export namespace PullRequestReviewCommon {
462478
}
463479
await folderRepositoryManager.checkoutDefaultBranch(defaultBranch, item);
464480
}
465-
await folderRepositoryManager.repository.deleteBranch(branchInfo!.branch, true);
481+
try {
482+
await folderRepositoryManager.repository.deleteBranch(branchInfo!.branch, true);
483+
} catch (error) {
484+
if (!isBranchNotFoundError(error)) {
485+
throw error;
486+
}
487+
Logger.debug(`Local branch ${branchInfo!.branch} no longer exists.`, 'PullRequestReviewCommon');
488+
}
466489
return deletedBranchTypes.push(action.type);
467490
case 'remote':
468491
deletedBranchTypes.push(action.type);

0 commit comments

Comments
 (0)