Skip to content

Commit 4cb17f1

Browse files
Copilotalexr00
andcommitted
Refactor to reuse deleteBranch logic via shared performBranchDeletion helper
Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>
1 parent f5d0f99 commit 4cb17f1

2 files changed

Lines changed: 68 additions & 31 deletions

File tree

src/@types/vscode.proposed.chatParticipantAdditions.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ declare module 'vscode' {
105105
isComplete?: boolean;
106106
toolSpecificData?: ChatTerminalToolInvocationData;
107107
fromSubAgent?: boolean;
108+
presentation?: 'hidden' | 'hiddenAfterComplete' | undefined;
108109

109110
constructor(toolName: string, toolCallId: string, isError?: boolean);
110111
}

src/github/pullRequestReviewCommon.ts

Lines changed: 67 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -350,15 +350,11 @@ export namespace PullRequestReviewCommon {
350350
const promises = selectedActions.map(async action => {
351351
switch (action.type) {
352352
case 'remoteHead':
353-
await folderRepositoryManager.deleteBranch(item);
353+
await performBranchDeletion(folderRepositoryManager, item, branchInfo, defaultBranch, 'remoteHead');
354354
deletedBranchTypes.push(action.type);
355-
await folderRepositoryManager.repository.fetch({ prune: true });
356-
// If we're in a remote repository, then we should checkout the default branch.
357-
if (folderRepositoryManager.repository.rootUri.scheme === Schemes.VscodeVfs) {
358-
await folderRepositoryManager.repository.checkout(defaultBranch);
359-
}
360355
return;
361356
case 'local':
357+
// Interactive deletion has special handling for dirty working tree
362358
if (isBranchActiveForDeletion) {
363359
if (folderRepositoryManager.repository.state.workingTreeChanges.length) {
364360
const yes = vscode.l10n.t('Yes');
@@ -373,16 +369,18 @@ export namespace PullRequestReviewCommon {
373369
return;
374370
}
375371
}
376-
await folderRepositoryManager.checkoutDefaultBranch(defaultBranch);
377372
}
378-
await folderRepositoryManager.repository.deleteBranch(branchInfo!.branch, true);
379-
return deletedBranchTypes.push(action.type);
373+
await performBranchDeletion(folderRepositoryManager, item, branchInfo, defaultBranch, 'local', isBranchActiveForDeletion);
374+
deletedBranchTypes.push(action.type);
375+
return;
380376
case 'remote':
377+
await performBranchDeletion(folderRepositoryManager, item, branchInfo, defaultBranch, 'remote');
381378
deletedBranchTypes.push(action.type);
382-
return folderRepositoryManager.repository.removeRemote(branchInfo!.remote!);
379+
return;
383380
case 'suspend':
381+
await performBranchDeletion(folderRepositoryManager, item, branchInfo, defaultBranch, 'suspend');
384382
deletedBranchTypes.push(action.type);
385-
return vscode.commands.executeCommand('github.codespaces.disconnectSuspend');
383+
return;
386384
}
387385
});
388386

@@ -413,6 +411,47 @@ export namespace PullRequestReviewCommon {
413411
(folderRepositoryManager.repository.state.HEAD?.name === branchName);
414412
}
415413

414+
/**
415+
* Core branch deletion logic shared between interactive and automatic deletion.
416+
* @param folderRepositoryManager The folder repository manager
417+
* @param item The pull request model
418+
* @param branchInfo Branch information for the pull request
419+
* @param defaultBranch The default branch name
420+
* @param actionType The type of deletion action to perform
421+
* @param checkoutBeforeDelete If true, checkout default branch before deleting local branch (for active branches)
422+
*/
423+
async function performBranchDeletion(
424+
folderRepositoryManager: FolderRepositoryManager,
425+
item: PullRequestModel,
426+
branchInfo: { branch: string; remote?: string; createdForPullRequest?: boolean; remoteInUse?: boolean } | undefined,
427+
defaultBranch: string,
428+
actionType: 'remoteHead' | 'local' | 'remote' | 'suspend',
429+
checkoutBeforeDelete: boolean = false
430+
): Promise<void> {
431+
switch (actionType) {
432+
case 'remoteHead':
433+
await folderRepositoryManager.deleteBranch(item);
434+
await folderRepositoryManager.repository.fetch({ prune: true });
435+
// If we're in a remote repository, then we should checkout the default branch.
436+
if (folderRepositoryManager.repository.rootUri.scheme === Schemes.VscodeVfs) {
437+
await folderRepositoryManager.repository.checkout(defaultBranch);
438+
}
439+
break;
440+
case 'local':
441+
if (checkoutBeforeDelete) {
442+
await folderRepositoryManager.checkoutDefaultBranch(defaultBranch);
443+
}
444+
await folderRepositoryManager.repository.deleteBranch(branchInfo!.branch, true);
445+
break;
446+
case 'remote':
447+
await folderRepositoryManager.repository.removeRemote(branchInfo!.remote!);
448+
break;
449+
case 'suspend':
450+
await vscode.commands.executeCommand('github.codespaces.disconnectSuspend');
451+
break;
452+
}
453+
}
454+
416455
/**
417456
* Automatically delete branches after merge based on user preferences.
418457
* This function does not show any prompts - it uses the default deletion method preferences.
@@ -430,41 +469,38 @@ export namespace PullRequestReviewCommon {
430469
.getConfiguration(PR_SETTINGS_NAMESPACE)
431470
.get<boolean>(`${DEFAULT_DELETION_METHOD}.${SELECT_REMOTE}`, true);
432471

433-
const deletionTasks: Promise<void>[] = [];
472+
// Build list of actions to execute based on preferences
473+
const actions: Array<{ type: 'remoteHead' | 'local' | 'remote'; checkoutFirst?: boolean }> = [];
434474

435475
// Delete remote head branch if it's not the default branch
436476
if (item.isResolved()) {
437477
const isDefaultBranch = defaultBranch === item.head.ref;
438478
if (!isDefaultBranch && !item.isRemoteHeadDeleted) {
439-
deletionTasks.push(
440-
folderRepositoryManager.deleteBranch(item)
441-
.then(() => folderRepositoryManager.repository.fetch({ prune: true }))
442-
.catch(e => Logger.warn(`Failed to delete remote branch for PR #${item.number}: ${e}`, 'PullRequestReviewCommon'))
443-
);
479+
actions.push({ type: 'remoteHead' });
444480
}
445481
}
446482

447483
// Delete local branch if preference is set
448484
if (branchInfo && deleteLocalBranch) {
449-
deletionTasks.push(
450-
(async () => {
451-
if (isBranchActive(folderRepositoryManager, item, branchInfo.branch)) {
452-
await folderRepositoryManager.checkoutDefaultBranch(defaultBranch);
453-
}
454-
await folderRepositoryManager.repository.deleteBranch(branchInfo.branch, true);
455-
})().catch(e => Logger.warn(`Failed to delete local branch ${branchInfo.branch} for PR #${item.number}: ${e}`, 'PullRequestReviewCommon'))
456-
);
485+
const needsCheckout = isBranchActive(folderRepositoryManager, item, branchInfo.branch);
486+
actions.push({ type: 'local', checkoutFirst: needsCheckout });
457487
}
458488

459489
// Delete remote if it's no longer used and preference is set
460490
if (branchInfo && branchInfo.remote && branchInfo.createdForPullRequest && !branchInfo.remoteInUse && deleteRemote) {
461-
deletionTasks.push(
462-
folderRepositoryManager.repository.removeRemote(branchInfo.remote)
463-
.catch(e => Logger.warn(`Failed to delete remote ${branchInfo.remote} for PR #${item.number}: ${e}`, 'PullRequestReviewCommon'))
464-
);
491+
actions.push({ type: 'remote' });
465492
}
466493

467-
// Execute all deletions in parallel
468-
await Promise.all(deletionTasks);
494+
// Execute deletions with error handling
495+
try {
496+
await Promise.all(
497+
actions.map(action =>
498+
performBranchDeletion(folderRepositoryManager, item, branchInfo, defaultBranch, action.type, action.checkoutFirst)
499+
.catch(e => Logger.warn(`Failed to delete ${action.type} for PR #${item.number}: ${e}`, 'PullRequestReviewCommon'))
500+
)
501+
);
502+
} catch (e) {
503+
Logger.warn(`Failed to auto-delete branches for PR #${item.number}: ${e}`, 'PullRequestReviewCommon');
504+
}
469505
}
470506
}

0 commit comments

Comments
 (0)