Skip to content

Commit be8587f

Browse files
committed
Revert "Address code review feedback: extract helper and improve error handling"
This reverts commit f5d0f99.
1 parent 00b52af commit be8587f

2 files changed

Lines changed: 24 additions & 22 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: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ export namespace PullRequestReviewCommon {
345345
const deletedBranchTypes: string[] = [];
346346

347347
if (selectedActions) {
348-
const isBranchActiveForDeletion = branchInfo && isBranchActive(folderRepositoryManager, item, branchInfo.branch);
348+
const isBranchActive = item.equals(folderRepositoryManager.activePullRequest) || (folderRepositoryManager.repository.state.HEAD?.name && folderRepositoryManager.repository.state.HEAD.name === branchInfo?.branch);
349349

350350
const promises = selectedActions.map(async action => {
351351
switch (action.type) {
@@ -359,7 +359,7 @@ export namespace PullRequestReviewCommon {
359359
}
360360
return;
361361
case 'local':
362-
if (isBranchActiveForDeletion) {
362+
if (isBranchActive) {
363363
if (folderRepositoryManager.repository.state.workingTreeChanges.length) {
364364
const yes = vscode.l10n.t('Yes');
365365
const response = await vscode.window.showWarningMessage(
@@ -405,14 +405,6 @@ export namespace PullRequestReviewCommon {
405405
}
406406
}
407407

408-
/**
409-
* Check if a pull request's branch is currently active (checked out).
410-
*/
411-
function isBranchActive(folderRepositoryManager: FolderRepositoryManager, item: PullRequestModel, branchName: string): boolean {
412-
return item.equals(folderRepositoryManager.activePullRequest) ||
413-
(folderRepositoryManager.repository.state.HEAD?.name === branchName);
414-
}
415-
416408
/**
417409
* Automatically delete branches after merge based on user preferences.
418410
* This function does not show any prompts - it uses the default deletion method preferences.
@@ -430,41 +422,50 @@ export namespace PullRequestReviewCommon {
430422
.getConfiguration(PR_SETTINGS_NAMESPACE)
431423
.get<boolean>(`${DEFAULT_DELETION_METHOD}.${SELECT_REMOTE}`, true);
432424

433-
const deletionTasks: Promise<void>[] = [];
425+
const promises: Promise<void>[] = [];
434426

435427
// Delete remote head branch if it's not the default branch
436428
if (item.isResolved()) {
437429
const isDefaultBranch = defaultBranch === item.head.ref;
438430
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'))
431+
promises.push(
432+
folderRepositoryManager.deleteBranch(item).then(() => {
433+
return folderRepositoryManager.repository.fetch({ prune: true });
434+
}).catch(e => {
435+
Logger.warn(`Failed to delete remote branch for PR #${item.number}: ${e}`, 'PullRequestReviewCommon');
436+
})
443437
);
444438
}
445439
}
446440

447441
// Delete local branch if preference is set
448442
if (branchInfo && deleteLocalBranch) {
449-
deletionTasks.push(
443+
const isBranchActive = item.equals(folderRepositoryManager.activePullRequest) ||
444+
(folderRepositoryManager.repository.state.HEAD?.name && folderRepositoryManager.repository.state.HEAD.name === branchInfo.branch);
445+
446+
promises.push(
450447
(async () => {
451-
if (isBranchActive(folderRepositoryManager, item, branchInfo.branch)) {
448+
if (isBranchActive) {
449+
// Checkout default branch before deleting the active branch
452450
await folderRepositoryManager.checkoutDefaultBranch(defaultBranch);
453451
}
454452
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'))
453+
})().catch(e => {
454+
Logger.warn(`Failed to delete local branch ${branchInfo.branch} for PR #${item.number}: ${e}`, 'PullRequestReviewCommon');
455+
})
456456
);
457457
}
458458

459459
// Delete remote if it's no longer used and preference is set
460460
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'))
461+
promises.push(
462+
folderRepositoryManager.repository.removeRemote(branchInfo.remote).catch(e => {
463+
Logger.warn(`Failed to delete remote ${branchInfo.remote} for PR #${item.number}: ${e}`, 'PullRequestReviewCommon');
464+
})
464465
);
465466
}
466467

467468
// Execute all deletions in parallel
468-
await Promise.all(deletionTasks);
469+
await Promise.all(promises);
469470
}
470471
}

0 commit comments

Comments
 (0)