Skip to content

Commit 00b52af

Browse files
committed
Revert "Refactor to reuse deleteBranch logic via shared performBranchDeletion helper"
This reverts commit 4cb17f1.
1 parent 4cb17f1 commit 00b52af

2 files changed

Lines changed: 31 additions & 68 deletions

File tree

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

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

110109
constructor(toolName: string, toolCallId: string, isError?: boolean);
111110
}

src/github/pullRequestReviewCommon.ts

Lines changed: 31 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -350,11 +350,15 @@ export namespace PullRequestReviewCommon {
350350
const promises = selectedActions.map(async action => {
351351
switch (action.type) {
352352
case 'remoteHead':
353-
await performBranchDeletion(folderRepositoryManager, item, branchInfo, defaultBranch, 'remoteHead');
353+
await folderRepositoryManager.deleteBranch(item);
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+
}
355360
return;
356361
case 'local':
357-
// Interactive deletion has special handling for dirty working tree
358362
if (isBranchActiveForDeletion) {
359363
if (folderRepositoryManager.repository.state.workingTreeChanges.length) {
360364
const yes = vscode.l10n.t('Yes');
@@ -369,18 +373,16 @@ export namespace PullRequestReviewCommon {
369373
return;
370374
}
371375
}
376+
await folderRepositoryManager.checkoutDefaultBranch(defaultBranch);
372377
}
373-
await performBranchDeletion(folderRepositoryManager, item, branchInfo, defaultBranch, 'local', isBranchActiveForDeletion);
374-
deletedBranchTypes.push(action.type);
375-
return;
378+
await folderRepositoryManager.repository.deleteBranch(branchInfo!.branch, true);
379+
return deletedBranchTypes.push(action.type);
376380
case 'remote':
377-
await performBranchDeletion(folderRepositoryManager, item, branchInfo, defaultBranch, 'remote');
378381
deletedBranchTypes.push(action.type);
379-
return;
382+
return folderRepositoryManager.repository.removeRemote(branchInfo!.remote!);
380383
case 'suspend':
381-
await performBranchDeletion(folderRepositoryManager, item, branchInfo, defaultBranch, 'suspend');
382384
deletedBranchTypes.push(action.type);
383-
return;
385+
return vscode.commands.executeCommand('github.codespaces.disconnectSuspend');
384386
}
385387
});
386388

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

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-
455416
/**
456417
* Automatically delete branches after merge based on user preferences.
457418
* This function does not show any prompts - it uses the default deletion method preferences.
@@ -469,38 +430,41 @@ export namespace PullRequestReviewCommon {
469430
.getConfiguration(PR_SETTINGS_NAMESPACE)
470431
.get<boolean>(`${DEFAULT_DELETION_METHOD}.${SELECT_REMOTE}`, true);
471432

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

475435
// Delete remote head branch if it's not the default branch
476436
if (item.isResolved()) {
477437
const isDefaultBranch = defaultBranch === item.head.ref;
478438
if (!isDefaultBranch && !item.isRemoteHeadDeleted) {
479-
actions.push({ type: 'remoteHead' });
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+
);
480444
}
481445
}
482446

483447
// Delete local branch if preference is set
484448
if (branchInfo && deleteLocalBranch) {
485-
const needsCheckout = isBranchActive(folderRepositoryManager, item, branchInfo.branch);
486-
actions.push({ type: 'local', checkoutFirst: needsCheckout });
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+
);
487457
}
488458

489459
// Delete remote if it's no longer used and preference is set
490460
if (branchInfo && branchInfo.remote && branchInfo.createdForPullRequest && !branchInfo.remoteInUse && deleteRemote) {
491-
actions.push({ type: 'remote' });
492-
}
493-
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-
)
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'))
501464
);
502-
} catch (e) {
503-
Logger.warn(`Failed to auto-delete branches for PR #${item.number}: ${e}`, 'PullRequestReviewCommon');
504465
}
466+
467+
// Execute all deletions in parallel
468+
await Promise.all(deletionTasks);
505469
}
506470
}

0 commit comments

Comments
 (0)