Skip to content

Commit 8e75775

Browse files
committed
Pull Request explorer pane is not shown when it is hidden and opening PR from a URL
Fixes #8089
1 parent 39c6f5e commit 8e75775

File tree

5 files changed

+146
-131
lines changed

5 files changed

+146
-131
lines changed

src/commands.ts

Lines changed: 5 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,10 @@
77
import * as pathLib from 'path';
88
import * as vscode from 'vscode';
99
import { Repository } from './api/api';
10-
import { GitErrorCodes, Status } from './api/api1';
10+
import { GitErrorCodes } from './api/api1';
1111
import { CommentReply, findActiveHandler, resolveCommentHandler } from './commentHandlerResolver';
1212
import { commands } from './common/executeCommands';
1313
import Logger from './common/logger';
14-
import * as PersistentState from './common/persistentState';
1514
import { FILE_LIST_LAYOUT, HIDE_VIEWED_FILES, PR_SETTINGS_NAMESPACE } from './common/settingKeys';
1615
import { editQuery } from './common/settingsUtils';
1716
import { ITelemetry } from './common/telemetry';
@@ -51,81 +50,6 @@ import {
5150
import { PRNode } from './view/treeNodes/pullRequestNode';
5251
import { RepositoryChangesNode } from './view/treeNodes/repositoryChangesNode';
5352

54-
// Modal dialog options for handling uncommitted changes during PR checkout
55-
const STASH_CHANGES = vscode.l10n.t('Stash changes');
56-
const DISCARD_CHANGES = vscode.l10n.t('Discard changes');
57-
const DONT_SHOW_AGAIN = vscode.l10n.t('Try to checkout anyway and don\'t show again');
58-
59-
// Constants for persistent state storage
60-
const UNCOMMITTED_CHANGES_SCOPE = vscode.l10n.t('uncommitted changes warning');
61-
const UNCOMMITTED_CHANGES_STORAGE_KEY = 'showWarning';
62-
63-
/**
64-
* Shows a modal dialog when there are uncommitted changes during PR checkout
65-
* @param repository The git repository with uncommitted changes
66-
* @returns Promise<boolean> true if user chose to proceed (after staging/discarding), false if cancelled
67-
*/
68-
async function handleUncommittedChanges(repository: Repository): Promise<boolean> {
69-
// Check if user has disabled the warning using persistent state
70-
if (PersistentState.fetch(UNCOMMITTED_CHANGES_SCOPE, UNCOMMITTED_CHANGES_STORAGE_KEY) === false) {
71-
return true; // User has disabled warnings, proceed without showing dialog
72-
}
73-
74-
// Filter out untracked files as they typically don't conflict with PR checkout
75-
const trackedWorkingTreeChanges = repository.state.workingTreeChanges.filter(change => change.status !== Status.UNTRACKED);
76-
const hasTrackedWorkingTreeChanges = trackedWorkingTreeChanges.length > 0;
77-
const hasIndexChanges = repository.state.indexChanges.length > 0;
78-
79-
if (!hasTrackedWorkingTreeChanges && !hasIndexChanges) {
80-
return true; // No tracked uncommitted changes, proceed
81-
}
82-
83-
const modalResult = await vscode.window.showInformationMessage(
84-
vscode.l10n.t('You have uncommitted changes that might be overwritten by checking out this pull request.'),
85-
{
86-
modal: true,
87-
detail: vscode.l10n.t('Choose how to handle your uncommitted changes before checking out the pull request.'),
88-
},
89-
STASH_CHANGES,
90-
DISCARD_CHANGES,
91-
DONT_SHOW_AGAIN,
92-
);
93-
94-
if (!modalResult) {
95-
return false; // User cancelled
96-
}
97-
98-
if (modalResult === DONT_SHOW_AGAIN) {
99-
// Store preference to never show this dialog again using persistent state
100-
PersistentState.store(UNCOMMITTED_CHANGES_SCOPE, UNCOMMITTED_CHANGES_STORAGE_KEY, false);
101-
return true; // Proceed with checkout
102-
}
103-
104-
try {
105-
if (modalResult === STASH_CHANGES) {
106-
// Stash all changes (working tree changes + any unstaged changes)
107-
const allChangedFiles = [
108-
...trackedWorkingTreeChanges.map(change => change.uri.fsPath),
109-
...repository.state.indexChanges.map(change => change.uri.fsPath),
110-
];
111-
if (allChangedFiles.length > 0) {
112-
await repository.add(allChangedFiles);
113-
await vscode.commands.executeCommand('git.stash', repository);
114-
}
115-
} else if (modalResult === DISCARD_CHANGES) {
116-
// Discard all tracked working tree changes
117-
const trackedWorkingTreeFiles = trackedWorkingTreeChanges.map(change => change.uri.fsPath);
118-
if (trackedWorkingTreeFiles.length > 0) {
119-
await repository.clean(trackedWorkingTreeFiles);
120-
}
121-
}
122-
return true; // Successfully handled changes, proceed with checkout
123-
} catch (error) {
124-
vscode.window.showErrorMessage(vscode.l10n.t('Failed to handle uncommitted changes: {0}', formatError(error)));
125-
return false;
126-
}
127-
}
128-
12953
function ensurePR(folderRepoManager: FolderRepositoryManager, pr?: PRNode): PullRequestModel;
13054
function ensurePR<TIssue extends Issue, TIssueModel extends IssueModel<TIssue>>(folderRepoManager: FolderRepositoryManager, pr?: TIssueModel): TIssueModel;
13155
function ensurePR<TIssue extends Issue, TIssueModel extends IssueModel<TIssue>>(folderRepoManager: FolderRepositoryManager, pr?: PRNode | TIssueModel): TIssueModel {
@@ -493,37 +417,6 @@ export function registerCommands(
493417
),
494418
);
495419

496-
const switchToPr = async (folderManager: FolderRepositoryManager, pullRequestModel: PullRequestModel, repository: Repository | undefined, isFromDescription: boolean) => {
497-
// If we don't have a repository from the node, use the one from the folder manager
498-
const repositoryToCheck = repository || folderManager.repository;
499-
500-
// Check for uncommitted changes before proceeding with checkout
501-
const shouldProceed = await handleUncommittedChanges(repositoryToCheck);
502-
if (!shouldProceed) {
503-
return; // User cancelled or there was an error handling changes
504-
}
505-
506-
/* __GDPR__
507-
"pr.checkout" : {
508-
"fromDescriptionPage" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" }
509-
}
510-
*/
511-
telemetry.sendTelemetryEvent('pr.checkout', { fromDescription: isFromDescription.toString() });
512-
513-
return vscode.window.withProgress(
514-
{
515-
location: vscode.ProgressLocation.SourceControl,
516-
title: vscode.l10n.t('Switching to Pull Request #{0}', pullRequestModel.number),
517-
},
518-
async () => {
519-
await ReviewManager.getReviewManagerForRepository(
520-
reviewsManager.reviewManagers,
521-
pullRequestModel.githubRepository,
522-
repository
523-
)?.switch(pullRequestModel);
524-
});
525-
};
526-
527420
context.subscriptions.push(
528421
vscode.commands.registerCommand('pr.pick', async (pr: PRNode | RepositoryChangesNode | PullRequestModel) => {
529422
if (pr === undefined) {
@@ -549,7 +442,7 @@ export function registerCommands(
549442
}
550443

551444
const fromDescriptionPage = pr instanceof PullRequestModel;
552-
return switchToPr(folderManager, pullRequestModel, repository, fromDescriptionPage);
445+
return reviewsManager.switchToPr(folderManager, pullRequestModel, repository, fromDescriptionPage);
553446

554447
}));
555448

@@ -653,14 +546,14 @@ export function registerCommands(
653546
return vscode.window.showErrorMessage(vscode.l10n.t('Unable to find pull request #{0}', prNumber.toString()));
654547
}
655548

656-
return switchToPr(folderManager, pullRequest, folderManager.repository, true);
549+
return reviewsManager.switchToPr(folderManager, pullRequest, folderManager.repository, true);
657550
}
658551

659552
const resolved = await resolvePr(ctx);
660553
if (!resolved) {
661554
return vscode.window.showErrorMessage(vscode.l10n.t('Unable to resolve pull request for checkout.'));
662555
}
663-
return switchToPr(resolved.folderManager, resolved.pr, resolved.folderManager.repository, true);
556+
return reviewsManager.switchToPr(resolved.folderManager, resolved.pr, resolved.folderManager.repository, true);
664557

665558
}));
666559

@@ -1035,7 +928,7 @@ export function registerCommands(
1035928
return vscode.window.showErrorMessage(vscode.l10n.t('Unable to find repository for pull request #{0}', pr.number.toString()));
1036929
}
1037930

1038-
return switchToPr(folderManager, pr, folderManager.repository, false);
931+
return reviewsManager.switchToPr(folderManager, pr, folderManager.repository, false);
1039932
}
1040933

1041934
async function closeChatSessionPullRequest(argument: ChatSessionWithPR | CrossChatSessionWithPR) {

src/extension.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,10 +268,10 @@ async function init(
268268
registerPostCommitCommandsProvider(reposManager, git);
269269

270270
// Resume any pending checkout request stored before workspace reopened.
271-
await resumePendingCheckout(context, reposManager);
271+
await resumePendingCheckout(reviewsManager, context, reposManager);
272272

273273
initChat(context, credentialStore, reposManager, copilotRemoteAgentManager, telemetry, prsTreeModel);
274-
context.subscriptions.push(vscode.window.registerUriHandler(new UriHandler(reposManager, telemetry, context, git)));
274+
context.subscriptions.push(vscode.window.registerUriHandler(new UriHandler(reposManager, reviewsManager, telemetry, context, git)));
275275

276276
// Make sure any compare changes tabs, which come from the create flow, are closed.
277277
CompareChanges.closeTabs();

src/uriHandler.ts

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,10 @@ import { ITelemetry } from './common/telemetry';
1010
import { fromOpenIssueWebviewUri, fromOpenOrCheckoutPullRequestWebviewUri, UriHandlerPaths } from './common/uri';
1111
import { FolderRepositoryManager } from './github/folderRepositoryManager';
1212
import { IssueOverviewPanel } from './github/issueOverview';
13+
import { PullRequestModel } from './github/pullRequestModel';
1314
import { PullRequestOverviewPanel } from './github/pullRequestOverview';
1415
import { RepositoriesManager } from './github/repositoriesManager';
16+
import { ReviewsManager } from './view/reviewsManager';
1517

1618
export const PENDING_CHECKOUT_PULL_REQUEST_KEY = 'pendingCheckoutPullRequest';
1719

@@ -25,7 +27,7 @@ interface PendingCheckoutPayload {
2527
function withCheckoutProgress<T>(owner: string, repo: string, prNumber: number, task: (progress: vscode.Progress<{ message?: string; increment?: number }>, token: vscode.CancellationToken) => Promise<T>): Promise<T> {
2628
return vscode.window.withProgress({
2729
location: vscode.ProgressLocation.Notification,
28-
title: vscode.l10n.t('Checking out pull request #{0} from {1}/{2}...', prNumber, owner, repo),
30+
title: vscode.l10n.t('Checking out pull request #{0} from {1}/{2}', prNumber, owner, repo),
2931
cancellable: true
3032
}, async (progress, token) => {
3133
if (token.isCancellationRequested) {
@@ -35,9 +37,13 @@ function withCheckoutProgress<T>(owner: string, repo: string, prNumber: number,
3537
}) as Promise<T>;
3638
}
3739

38-
async function performPullRequestCheckout(folderManager: FolderRepositoryManager, owner: string, repo: string, prNumber: number): Promise<void> {
40+
async function performPullRequestCheckout(reviewsManager: ReviewsManager, folderManager: FolderRepositoryManager, owner: string, repo: string, prNumber: number): Promise<void> {
3941
try {
40-
const pullRequest = await folderManager.resolvePullRequest(owner, repo, prNumber);
42+
let pullRequest: PullRequestModel | undefined;
43+
await withCheckoutProgress(owner, repo, prNumber, async (progress, _token) => {
44+
progress.report({ message: vscode.l10n.t('Resolving pull request') });
45+
pullRequest = await folderManager.resolvePullRequest(owner, repo, prNumber);
46+
});
4147
if (!pullRequest) {
4248
vscode.window.showErrorMessage(vscode.l10n.t('Pull request #{0} not found in {1}/{2}.', prNumber, owner, repo));
4349
Logger.warn(`Pull request #${prNumber} not found for checkout.`, UriHandler.ID);
@@ -49,13 +55,13 @@ async function performPullRequestCheckout(folderManager: FolderRepositoryManager
4955
return;
5056
}
5157

52-
await vscode.commands.executeCommand('pr.pick', pullRequest);
58+
await reviewsManager.switchToPr(folderManager, pullRequest, undefined, false);
5359
} catch (e) {
5460
Logger.error(`Error during pull request checkout: ${e instanceof Error ? e.message : String(e)}`, UriHandler.ID);
5561
}
5662
}
5763

58-
export async function resumePendingCheckout(context: vscode.ExtensionContext, reposManager: RepositoriesManager): Promise<void> {
64+
export async function resumePendingCheckout(reviewsManager: ReviewsManager, context: vscode.ExtensionContext, reposManager: RepositoriesManager): Promise<void> {
5965
const pending = context.globalState.get<PendingCheckoutPayload>(PENDING_CHECKOUT_PULL_REQUEST_KEY);
6066
if (!pending) {
6167
return;
@@ -72,7 +78,7 @@ export async function resumePendingCheckout(context: vscode.ExtensionContext, re
7278
if (!folderManager) {
7379
return false;
7480
}
75-
await performPullRequestCheckout(folderManager, pending.owner, pending.repo, pending.pullRequestNumber);
81+
await performPullRequestCheckout(reviewsManager, folderManager, pending.owner, pending.repo, pending.pullRequestNumber);
7682
await context.globalState.update(PENDING_CHECKOUT_PULL_REQUEST_KEY, undefined);
7783
return true;
7884
};
@@ -95,6 +101,7 @@ export async function showCheckoutPrompt(owner: string, repo: string, prNumber:
95101
export class UriHandler implements vscode.UriHandler {
96102
public static readonly ID = 'UriHandler';
97103
constructor(private readonly _reposManagers: RepositoriesManager,
104+
private readonly _reviewsManagers: ReviewsManager,
98105
private readonly _telemetry: ITelemetry,
99106
private readonly _context: vscode.ExtensionContext,
100107
private readonly _git: GitApiImpl
@@ -151,22 +158,22 @@ export class UriHandler implements vscode.UriHandler {
151158
}
152159
const folderManager = this._reposManagers.getManagerForRepository(params.owner, params.repo);
153160
if (folderManager) {
154-
return performPullRequestCheckout(folderManager, params.owner, params.repo, params.pullRequestNumber);
161+
return performPullRequestCheckout(this._reviewsManagers, folderManager, params.owner, params.repo, params.pullRequestNumber);
155162
}
156163
// Folder not found; request workspace open then resume later.
157164
await withCheckoutProgress(params.owner, params.repo, params.pullRequestNumber, async (progress, token) => {
158165
if (token.isCancellationRequested) {
159166
return;
160167
}
161168
try {
162-
progress.report({ message: vscode.l10n.t('Locating workspace...') });
169+
progress.report({ message: vscode.l10n.t('Locating workspace') });
163170
const remoteUri = vscode.Uri.parse(`https://github.com/${params.owner}/${params.repo}`);
164171
const workspaces = await this._git.getRepositoryWorkspace(remoteUri);
165172
if (token.isCancellationRequested) {
166173
return;
167174
}
168175
if (workspaces && workspaces.length) {
169-
progress.report({ message: vscode.l10n.t('Opening workspace...') });
176+
progress.report({ message: vscode.l10n.t('Opening workspace') });
170177
await this._savePendingCheckoutAndOpenFolder(params, workspaces[0]);
171178
} else {
172179
this._showCloneOffer(remoteUri, params);

src/view/reviewManager.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -381,13 +381,13 @@ export class ReviewManager extends Disposable {
381381
}
382382
}
383383

384-
private async resolvePullRequest(metadata: PullRequestMetadata): Promise<(PullRequestModel & IResolvedPullRequestModel) | undefined> {
384+
private async resolvePullRequest(metadata: PullRequestMetadata, useCache: boolean): Promise<(PullRequestModel & IResolvedPullRequestModel) | undefined> {
385385
try {
386386
this._prNumber = metadata.prNumber;
387387

388388
const { owner, repositoryName } = metadata;
389389
Logger.appendLine('Resolving pull request', this.id);
390-
let pr = await this._folderRepoManager.resolvePullRequest(owner, repositoryName, metadata.prNumber);
390+
let pr = await this._folderRepoManager.resolvePullRequest(owner, repositoryName, metadata.prNumber, useCache);
391391

392392
if (!pr || !pr.isResolved() || !(await pr.githubRepository.hasBranch(pr.base.name))) {
393393
await this.clear(true);
@@ -450,7 +450,9 @@ export class ReviewManager extends Disposable {
450450
`current branch ${this._repository.state.HEAD.name} is associated with pull request #${matchingPullRequestMetadata.prNumber}`, this.id
451451
);
452452
const previousPrNumber = this._prNumber;
453-
let pr = await this.resolvePullRequest(matchingPullRequestMetadata);
453+
// Use the cache if we just checked out the same PR as a small performance optimization.
454+
const justCheckedOutSamePr = this.justSwitchedToReviewMode && (previousPrNumber === matchingPullRequestMetadata.prNumber);
455+
let pr = await this.resolvePullRequest(matchingPullRequestMetadata, justCheckedOutSamePr);
454456
if (!pr) {
455457
Logger.appendLine(`Unable to resolve PR #${matchingPullRequestMetadata.prNumber}`, this.id);
456458
return;
@@ -461,15 +463,15 @@ export class ReviewManager extends Disposable {
461463
if (pr.state !== GithubItemStateEnum.Open) {
462464
const metadataFromGithub = await this.checkGitHubForPrBranch(branch);
463465
if (metadataFromGithub && metadataFromGithub?.prNumber !== pr.number) {
464-
const prFromGitHub = await this.resolvePullRequest(metadataFromGithub);
466+
const prFromGitHub = await this.resolvePullRequest(metadataFromGithub, false);
465467
if (prFromGitHub) {
466468
pr = prFromGitHub;
467469
}
468470
}
469471
}
470472

471473
const hasPushedChanges = branch.commit !== oldLastCommitSha && branch.ahead === 0 && branch.behind === 0;
472-
if (previousPrNumber === pr.number && !hasPushedChanges && (this._isShowingLastReviewChanges === pr.showChangesSinceReview)) {
474+
if (!this.justSwitchedToReviewMode && (previousPrNumber === pr.number) && !hasPushedChanges && (this._isShowingLastReviewChanges === pr.showChangesSinceReview)) {
473475
return;
474476
}
475477
this._isShowingLastReviewChanges = pr.showChangesSinceReview;
@@ -541,7 +543,9 @@ export class ReviewManager extends Disposable {
541543
})
542544
);
543545
Logger.appendLine(`Register in memory content provider`, this.id);
544-
await this.registerGitHubInMemContentProvider();
546+
if (previousPrNumber !== pr.number) {
547+
await this.registerGitHubInMemContentProvider();
548+
}
545549

546550
this.statusBarItem.text = '$(git-pull-request) ' + vscode.l10n.t('Pull Request #{0}', pr.number);
547551
this.statusBarItem.command = {

0 commit comments

Comments
 (0)