Improves getting the associated pr with a branch

Removes `getPullRequestForBranch` from GitProviderService as it doesn't belong there

src/commands/openAssociatedPullRequestOnRemote.ts

@@ -19,6 +19,7 @@ export class OpenAssociatedPullRequestOnRemoteCommand extends ActiveEditorComman
 
 		const gitUri = uri != null ? await GitUri.fromUri(uri) : undefined;
 
+		let args: OpenPullRequestOnRemoteCommandArgs;
 		if (editor != null && gitUri != null) {
 			const blameline = editor.selection.active.line;
 			if (blameline < 0) return;
@@ -27,46 +28,31 @@ export class OpenAssociatedPullRequestOnRemoteCommand extends ActiveEditorComman
 				const blame = await this.container.git.getBlameForLine(gitUri, blameline);
 				if (blame == null) return;
 
-				await executeCommand<OpenPullRequestOnRemoteCommandArgs>(Commands.OpenPullRequestOnRemote, {
-					clipboard: false,
-					ref: blame.commit.sha,
-					repoPath: blame.commit.repoPath,
-				});
+				args = { clipboard: false, ref: blame.commit.sha, repoPath: blame.commit.repoPath };
 			} catch (ex) {
 				Logger.error(ex, 'OpenAssociatedPullRequestOnRemoteCommand', `getBlameForLine(${blameline})`);
+				return;
 			}
+		} else {
+			try {
+				const repo = await getRepositoryOrShowPicker('Open Associated Pull Request', undefined, undefined, {
+					filter: async r => (await this.container.git.getBestRemoteWithRichProvider(r.uri)) != null,
+				});
+				if (repo == null) return;
 
-			return;
-		}
-
-		try {
-			const repo = await getRepositoryOrShowPicker('Open Pull Request Associated', undefined, undefined, {
-				filter: async r => (await this.container.git.getBestRemoteWithRichProvider(r.uri))?.provider != null,
-			});
-			if (repo == null) return;
-
-			const remote = await this.container.git.getBestRemoteWithRichProvider(repo.uri);
-			if (remote?.provider == null) return;
-
-			const branch = await repo.getBranch();
-			if (branch == null) return;
-
-			let pr = await this.container.git.getPullRequestForBranch(branch.ref, remote.provider);
-			if (pr == null) {
-				const commit = await repo.getCommit('HEAD');
-				if (commit == null) return;
+				const branch = await repo?.getBranch();
+				const pr = await branch?.getAssociatedPullRequest();
 
-				pr = await commit.getAssociatedPullRequest(remote);
-				if (pr == null) return;
+				args =
+					pr != null
+						? { clipboard: false, pr: { url: pr.url } }
+						: { clipboard: false, ref: branch?.name ?? 'HEAD', repoPath: repo.path };
+			} catch (ex) {
+				Logger.error(ex, 'OpenAssociatedPullRequestOnRemoteCommand', 'No editor opened');
+				return;
 			}
-
-			await executeCommand<OpenPullRequestOnRemoteCommandArgs>(Commands.OpenPullRequestOnRemote, {
-				pr: {
-					url: pr.url,
-				},
-			});
-		} catch (ex) {
-			Logger.error(ex, 'OpenAssociatedPullRequestOnRemoteCommand', 'No editor opened');
 		}
+
+		await executeCommand<OpenPullRequestOnRemoteCommandArgs>(Commands.OpenPullRequestOnRemote, args);
 	}
 }

src/commands/openPullRequestOnRemote.ts

@@ -1,6 +1,7 @@
-import { env, Uri } from 'vscode';
+import { env, Uri, window } from 'vscode';
 import { Commands } from '../constants';
 import type { Container } from '../container';
+import { shortenRevision } from '../git/models/reference';
 import { command } from '../system/command';
 import { PullRequestNode } from '../views/nodes/pullRequestNode';
 import type { CommandContext } from './base';
@@ -39,7 +40,10 @@ export class OpenPullRequestOnRemoteCommand extends Command {
 			if (!remote?.hasRichIntegration()) return;
 
 			const pr = await remote.provider.getPullRequestForCommit(args.ref);
-			if (pr == null) return;
+			if (pr == null) {
+				void window.showInformationMessage(`No pull request associated with '${shortenRevision(args.ref)}'`);
+				return;
+			}
 
 			args = { ...args };
 			args.pr = pr;

src/env/node/git/localGitProvider.ts

@@ -1753,6 +1753,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
 			]);
 
 			branch = new GitBranch(
+				this.container,
 				repoPath,
 				rebaseStatus?.incoming.name ?? name,
 				false,
@@ -1801,6 +1802,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
 							]);
 
 							current = new GitBranch(
+								this.container,
 								repoPath!,
 								rebaseStatus?.incoming.name ?? name,
 								false,
@@ -1818,7 +1820,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
 						return current != null ? { values: [current] } : emptyPagedResult;
 					}
 
-					return { values: GitBranchParser.parse(data, repoPath!) };
+					return { values: GitBranchParser.parse(this.container, data, repoPath!) };
 				} catch (ex) {
 					this._branchesCache.delete(repoPath!);
 

src/git/gitProviderService.ts

@@ -63,7 +63,7 @@ import type { GitGraph } from './models/graph';
 import type { SearchedIssue } from './models/issue';
 import type { GitLog } from './models/log';
 import type { GitMergeStatus } from './models/merge';
-import type { PullRequest, PullRequestState, SearchedPullRequest } from './models/pullRequest';
+import type { SearchedPullRequest } from './models/pullRequest';
 import type { GitRebaseStatus } from './models/rebase';
 import type { GitBranchReference, GitReference } from './models/reference';
 import { createRevisionRange, isSha, isUncommitted, isUncommittedParent } from './models/reference';
@@ -1925,59 +1925,6 @@ export class GitProviderService implements Disposable {
 		return provider.getPreviousComparisonUrisForLine(path, uri, editorLine, ref, skip);
 	}
 
-	async getPullRequestForBranch(
-		branch: string,
-		remote: GitRemote<RemoteProvider | RichRemoteProvider>,
-		options?: { avatarSize?: number; include?: PullRequestState[]; limit?: number; timeout?: number },
-	): Promise<PullRequest | undefined>;
-	async getPullRequestForBranch(
-		branch: string,
-		provider: RichRemoteProvider,
-		options?: { avatarSize?: number; include?: PullRequestState[]; limit?: number; timeout?: number },
-	): Promise<PullRequest | undefined>;
-	@gate<GitProviderService['getPullRequestForBranch']>((branch, remoteOrProvider, options) => {
-		const provider = GitRemote.is(remoteOrProvider) ? remoteOrProvider.provider : remoteOrProvider;
-		return `${branch}${
-			provider != null ? `|${provider.id}:${provider.domain}/${provider.path}` : ''
-		}|${JSON.stringify(options)}`;
-	})
-	@debug<GitProviderService['getPullRequestForBranch']>({ args: { 1: remoteOrProvider => remoteOrProvider.name } })
-	async getPullRequestForBranch(
-		branch: string,
-		remoteOrProvider: GitRemote | RichRemoteProvider,
-		options?: { avatarSize?: number; include?: PullRequestState[]; limit?: number; timeout?: number },
-	): Promise<PullRequest | undefined> {
-		let provider;
-		if (GitRemote.is(remoteOrProvider)) {
-			({ provider } = remoteOrProvider);
-			if (!provider?.hasRichIntegration()) return undefined;
-		} else {
-			provider = remoteOrProvider;
-		}
-
-		let timeout;
-		if (options != null) {
-			({ timeout, ...options } = options);
-		}
-
-		let promiseOrPR = provider.getPullRequestForBranch(branch, options);
-		if (promiseOrPR == null || !isPromise(promiseOrPR)) {
-			return promiseOrPR;
-		}
-
-		if (timeout != null && timeout > 0) {
-			promiseOrPR = cancellable(promiseOrPR, timeout);
-		}
-
-		try {
-			return await promiseOrPR;
-		} catch (ex) {
-			if (ex instanceof PromiseCancelledError) throw ex;
-
-			return undefined;
-		}
-	}
-
 	@debug<GitProviderService['getMyPullRequests']>({ args: { 0: remoteOrProvider => remoteOrProvider.name } })
 	async getMyPullRequests(
 		remoteOrProvider: GitRemote | RichRemoteProvider,
@@ -2069,7 +2016,7 @@ export class GitProviderService implements Disposable {
 	async getBestRemoteWithProvider(
 		repoPath: string | Uri,
 		cancellation?: CancellationToken,
-	): Promise<GitRemote<RemoteProvider | RichRemoteProvider> | undefined> {
+	): Promise<GitRemote<RemoteProvider> | undefined> {
 		const remotes = await this.getBestRemotesWithProviders(repoPath, cancellation);
 		return remotes[0];
 	}
@@ -2078,7 +2025,7 @@ export class GitProviderService implements Disposable {
 	async getBestRemotesWithProviders(
 		repoPath: string | Uri,
 		cancellation?: CancellationToken,
-	): Promise<GitRemote<RemoteProvider | RichRemoteProvider>[]> {
+	): Promise<GitRemote<RemoteProvider>[]> {
 		if (repoPath == null) return [];
 		if (typeof repoPath === 'string') {
 			repoPath = this.getAbsoluteUri(repoPath);
@@ -2097,7 +2044,7 @@ export class GitProviderService implements Disposable {
 				const defaultRemote = remotes.find(r => r.default)?.name;
 				const currentBranchRemote = (await this.getBranch(remotes[0].repoPath))?.getRemoteName();
 
-				const weighted: [number, GitRemote<RemoteProvider | RichRemoteProvider>][] = [];
+				const weighted: [number, GitRemote<RemoteProvider>][] = [];
 
 				let originalFound = false;
 
@@ -2195,11 +2142,19 @@ export class GitProviderService implements Disposable {
 		repoPath: string | Uri,
 		options?: { sort?: boolean },
 		cancellation?: CancellationToken,
-	): Promise<GitRemote<RemoteProvider | RichRemoteProvider>[]> {
+	): Promise<GitRemote<RemoteProvider>[]> {
 		const remotes = await this.getRemotes(repoPath, options, cancellation);
-		return remotes.filter(
-			(r: GitRemote): r is GitRemote<RemoteProvider | RichRemoteProvider> => r.provider != null,
-		);
+		return remotes.filter((r: GitRemote): r is GitRemote<RemoteProvider> => r.provider != null);
+	}
+
+	@log()
+	async getRemotesWithRichProviders(
+		repoPath: string | Uri,
+		options?: { sort?: boolean },
+		cancellation?: CancellationToken,
+	): Promise<GitRemote<RichRemoteProvider>[]> {
+		const remotes = await this.getRemotes(repoPath, options, cancellation);
+		return remotes.filter((r: GitRemote): r is GitRemote<RichRemoteProvider> => r.hasRichIntegration());
 	}
 
 	getBestRepository(): Repository | undefined;

src/git/models/branch.ts

@@ -1,14 +1,11 @@
 import { BranchSorting, DateStyle } from '../../config';
-import { Container } from '../../container';
+import type { Container } from '../../container';
 import { configuration } from '../../system/configuration';
 import { formatDate, fromNow } from '../../system/date';
 import { debug } from '../../system/decorators/log';
 import { memoize } from '../../system/decorators/memoize';
 import { getLoggableName } from '../../system/logger';
-import { cancellable } from '../../system/promise';
 import { sortCompare } from '../../system/string';
-import type { RemoteProvider } from '../remotes/remoteProvider';
-import type { RichRemoteProvider } from '../remotes/richRemoteProvider';
 import type { PullRequest, PullRequestState } from './pullRequest';
 import type { GitBranchReference, GitReference } from './reference';
 import { getBranchTrackingWithoutRemote, shortenRevision } from './reference';
@@ -53,6 +50,7 @@ export class GitBranch implements GitBranchReference {
 	readonly state: GitTrackingState;
 
 	constructor(
+		private readonly container: Container,
 		public readonly repoPath: string,
 		public readonly name: string,
 		public readonly remote: boolean,
@@ -84,8 +82,8 @@ export class GitBranch implements GitBranchReference {
 	}
 
 	get formattedDate(): string {
-		return Container.instance.BranchDateFormatting.dateStyle === DateStyle.Absolute
-			? this.formatDate(Container.instance.BranchDateFormatting.dateFormat)
+		return this.container.BranchDateFormatting.dateStyle === DateStyle.Absolute
+			? this.formatDate(this.container.BranchDateFormatting.dateFormat)
 			: this.formatDateFromNow();
 	}
 
@@ -102,27 +100,18 @@ export class GitBranch implements GitBranchReference {
 		return this.date != null ? fromNow(this.date) : '';
 	}
 
-	private _pullRequest: Promise<PullRequest | undefined> | undefined;
-
 	@debug()
 	async getAssociatedPullRequest(options?: {
 		avatarSize?: number;
 		include?: PullRequestState[];
-		limit?: number;
-		timeout?: number;
 	}): Promise<PullRequest | undefined> {
-		if (this._pullRequest == null) {
-			async function getCore(this: GitBranch): Promise<PullRequest | undefined> {
-				const remote = await this.getRemoteWithProvider();
-				if (remote == null) return undefined;
-
-				const branch = this.getTrackingWithoutRemote() ?? this.getNameWithoutRemote();
-				return Container.instance.git.getPullRequestForBranch(branch, remote, options);
-			}
-			this._pullRequest = getCore.call(this);
-		}
-
-		return cancellable(this._pullRequest, options?.timeout);
+		const remote = await this.getRemote();
+		return remote?.hasRichIntegration()
+			? remote.provider.getPullRequestForBranch(
+					this.getTrackingWithoutRemote() ?? this.getNameWithoutRemote(),
+					options,
+			  )
+			: undefined;
 	}
 
 	@memoize()
@@ -147,21 +136,8 @@ export class GitBranch implements GitBranchReference {
 		const remoteName = this.getRemoteName();
 		if (remoteName == null) return undefined;
 
-		const remotes = await Container.instance.git.getRemotes(this.repoPath);
-		if (remotes.length === 0) return undefined;
-
-		return remotes.find(r => r.name === remoteName);
-	}
-
-	@memoize()
-	async getRemoteWithProvider(): Promise<GitRemote<RemoteProvider | RichRemoteProvider> | undefined> {
-		const remoteName = this.getRemoteName();
-		if (remoteName == null) return undefined;
-
-		const remotes = await Container.instance.git.getRemotesWithProviders(this.repoPath);
-		if (remotes.length === 0) return undefined;
-
-		return remotes.find(r => r.name === remoteName);
+		const remotes = await this.container.git.getRemotes(this.repoPath);
+		return remotes.length ? remotes.find(r => r.name === remoteName) : undefined;
 	}
 
 	@memoize()
@@ -184,10 +160,9 @@ export class GitBranch implements GitBranchReference {
 			return GitBranchStatus.UpToDate;
 		}
 
-		const remotes = await Container.instance.git.getRemotesWithProviders(this.repoPath);
-		if (remotes.length > 0) return GitBranchStatus.Unpublished;
-
-		return GitBranchStatus.Local;
+		// If there are any remotes then say this is unpublished, otherwise local
+		const remotes = await this.container.git.getRemotes(this.repoPath);
+		return remotes.length ? GitBranchStatus.Unpublished : GitBranchStatus.Local;
 	}
 
 	getTrackingStatus(options?: {
@@ -203,16 +178,16 @@ export class GitBranch implements GitBranchReference {
 	}
 
 	get starred() {
-		const starred = Container.instance.storage.getWorkspace('starred:branches');
+		const starred = this.container.storage.getWorkspace('starred:branches');
 		return starred !== undefined && starred[this.id] === true;
 	}
 
 	star() {
-		return Container.instance.git.getRepository(this.repoPath)?.star(this);
+		return this.container.git.getRepository(this.repoPath)?.star(this);
 	}
 
 	unstar() {
-		return Container.instance.git.getRepository(this.repoPath)?.unstar(this);
+		return this.container.git.getRepository(this.repoPath)?.unstar(this);
 	}
 }
 

src/git/parsers/branchParser.ts

@@ -1,3 +1,4 @@
+import type { Container } from '../../container';
 import { debug } from '../../system/decorators/log';
 import { GitBranch } from '../models/branch';
 
@@ -20,7 +21,7 @@ export class GitBranchParser {
 	].join('');
 
 	@debug({ args: false, singleLine: true })
-	static parse(data: string, repoPath: string): GitBranch[] {
+	static parse(container: Container, data: string, repoPath: string): GitBranch[] {
 		const branches: GitBranch[] = [];
 
 		if (!data) return branches;
@@ -57,6 +58,7 @@ export class GitBranchParser {
 
 			branches.push(
 				new GitBranch(
+					container,
 					repoPath,
 					name,
 					remote,

src/plus/github/githubGitProvider.ts

@@ -917,11 +917,28 @@ export class GitHubGitProvider implements GitProvider, Disposable {
 							const ref = branch.target.oid;
 
 							branches.push(
-								new GitBranch(repoPath!, branch.name, false, branch.name === current, date, ref, {
-									name: `origin/${branch.name}`,
-									missing: false,
-								}),
-								new GitBranch(repoPath!, `origin/${branch.name}`, true, false, date, ref),
+								new GitBranch(
+									this.container,
+									repoPath!,
+									branch.name,
+									false,
+									branch.name === current,
+									date,
+									ref,
+									{
+										name: `origin/${branch.name}`,
+										missing: false,
+									},
+								),
+								new GitBranch(
+									this.container,
+									repoPath!,
+									`origin/${branch.name}`,
+									true,
+									false,
+									date,
+									ref,
+								),
 							);
 						}
 

src/views/nodes/repositoryNode.ts

@@ -83,6 +83,7 @@ export class RepositoryNode extends SubscribeableViewNode
 			const status = await this._status;
 			if (status != null) {
 				const branch = new GitBranch(
+					this.view.container,
 					status.repoPath,
 					status.branch,
 					false,

src/views/nodes/viewNode.ts

@@ -592,7 +592,7 @@ export abstract class RepositoryFolderNode<
 				);
 				providerName = providers?.length ? providers[0].name : undefined;
 			} else {
-				const remote = await branch.getRemoteWithProvider();
+				const remote = await branch.getRemote();
 				providerName = remote?.provider?.name;
 			}