Reworks getting the "best" remote with a provider

Improves best remote caching

src/annotations/lineAnnotationController.ts

@@ -162,7 +162,7 @@ export class LineAnnotationController implements Disposable {
 	) {
 		if (lines.length === 0) return undefined;
 
-		const remote = await this.container.git.getRichRemoteProvider(repoPath);
+		const remote = await this.container.git.getBestRemoteWithRichProvider(repoPath);
 		if (remote?.provider == null) return undefined;
 
 		const refs = new Set<string>();

src/avatars.ts

@@ -190,7 +190,7 @@ async function getAvatarUriFromRemoteProvider(
 			// 	account = await remote?.provider.getAccountForEmail(email, { avatarSize: size });
 			// } else {
 			if (typeof repoPathOrCommit !== 'string') {
-				const remote = await Container.instance.git.getRichRemoteProvider(repoPathOrCommit.repoPath);
+				const remote = await Container.instance.git.getBestRemoteWithRichProvider(repoPathOrCommit.repoPath);
 				account = await remote?.provider.getAccountForCommit(repoPathOrCommit.ref, { avatarSize: size });
 			}
 		}

src/commands/openPullRequestOnRemote.ts

@@ -34,7 +34,7 @@ export class OpenPullRequestOnRemoteCommand extends Command {
 		if (args?.pr == null) {
 			if (args?.repoPath == null || args?.ref == null) return;
 
-			const remote = await this.container.git.getRichRemoteProvider(args.repoPath);
+			const remote = await this.container.git.getBestRemoteWithRichProvider(args.repoPath);
 			if (remote?.provider == null) return;
 
 			const pr = await this.container.git.getPullRequestForCommit(args.ref, remote.provider);

src/commands/remoteProviders.ts

@@ -75,7 +75,7 @@ export class ConnectRemoteProviderCommand extends Command {
 		} else if (args?.remote == null) {
 			repoPath = args.repoPath;
 
-			remote = await this.container.git.getRichRemoteProvider(repoPath, { includeDisconnected: true });
+			remote = await this.container.git.getBestRemoteWithRichProvider(repoPath, { includeDisconnected: true });
 			if (remote == null) return false;
 		} else {
 			repoPath = args.repoPath;
@@ -166,7 +166,7 @@ export class DisconnectRemoteProviderCommand extends Command {
 		} else if (args?.remote == null) {
 			repoPath = args.repoPath;
 
-			remote = await this.container.git.getRichRemoteProvider(repoPath, { includeDisconnected: false });
+			remote = await this.container.git.getBestRemoteWithRichProvider(repoPath, { includeDisconnected: false });
 			if (remote == null) return undefined;
 		} else {
 			repoPath = args.repoPath;

src/git/gitProviderService.ts

@@ -166,7 +166,9 @@ export class GitProviderService implements Disposable {
 	private readonly _pendingRepositories = new Map<RepoComparisonKey, Promise<Repository | undefined>>();
 	private readonly _providers = new Map<GitProviderId, GitProvider>();
 	private readonly _repositories = new Repositories();
-	private readonly _richRemotesCache = new Map<string, GitRemote<RichRemoteProvider> | null>();
+	private readonly _bestRemotesCache: Map<RepoComparisonKey, GitRemote<RemoteProvider | RichRemoteProvider> | null> &
+		Map<`rich|${RepoComparisonKey}`, GitRemote<RichRemoteProvider> | null> &
+		Map<`rich+connected|${RepoComparisonKey}`, GitRemote<RichRemoteProvider> | null> = new Map();
 	private readonly _visitedPaths = new VisitedPathsTrie();
 
 	constructor(private readonly container: Container) {
@@ -358,7 +360,7 @@ export class GitProviderService implements Disposable {
 						RepositoryChangeComparisonMode.Any,
 					)
 				) {
-					this._richRemotesCache.clear();
+					this._bestRemotesCache.clear();
 				}
 
 				if (e.changed(RepositoryChange.Closed, RepositoryChangeComparisonMode.Any)) {
@@ -968,7 +970,7 @@ export class GitProviderService implements Disposable {
 		...affects: ('branches' | 'contributors' | 'providers' | 'remotes' | 'stashes' | 'status' | 'tags')[]
 	): void {
 		if (affects.length === 0 || affects.includes('providers')) {
-			this._richRemotesCache.clear();
+			this._bestRemotesCache.clear();
 		}
 
 		const repoAffects = affects.filter((c): c is 'branches' | 'remotes' => c === 'branches' || c === 'remotes');
@@ -1661,60 +1663,53 @@ export class GitProviderService implements Disposable {
 		return provider.getIncomingActivity(path, options);
 	}
 
-	async getRichRemoteProvider(
+	async getBestRemoteWithProvider(
 		repoPath: string | Uri | undefined,
-		options?: { includeDisconnected?: boolean },
-	): Promise<GitRemote<RichRemoteProvider> | undefined>;
-	async getRichRemoteProvider(
+	): Promise<GitRemote<RemoteProvider | RichRemoteProvider> | undefined>;
+	async getBestRemoteWithProvider(
 		remotes: GitRemote[],
-		options?: { includeDisconnected?: boolean },
-	): Promise<GitRemote<RichRemoteProvider> | undefined>;
-	@gate<GitProviderService['getRichRemoteProvider']>(
-		(remotesOrRepoPath, options) =>
+	): Promise<GitRemote<RemoteProvider | RichRemoteProvider> | undefined>;
+	@gate<GitProviderService['getBestRemoteWithProvider']>(
+		remotesOrRepoPath =>
 			`${
 				remotesOrRepoPath == null || typeof remotesOrRepoPath === 'string'
 					? remotesOrRepoPath
 					: remotesOrRepoPath instanceof Uri
 					? remotesOrRepoPath.toString()
-					: `${remotesOrRepoPath[0]?.repoPath}|${remotesOrRepoPath?.map(r => r.id).join(',') ?? ''}`
-			}|${options?.includeDisconnected ?? false}`,
+					: `${remotesOrRepoPath.length}:${remotesOrRepoPath[0]?.repoPath ?? ''}`
+			}`,
 	)
-	@log<GitProviderService['getRichRemoteProvider']>({
+	@log<GitProviderService['getBestRemoteWithProvider']>({
 		args: {
 			0: remotesOrRepoPath =>
 				Array.isArray(remotesOrRepoPath) ? remotesOrRepoPath.map(r => r.name).join(',') : remotesOrRepoPath,
 		},
 	})
-	async getRichRemoteProvider(
+	async getBestRemoteWithProvider(
 		remotesOrRepoPath: GitRemote[] | string | Uri | undefined,
-		options?: { includeDisconnected?: boolean },
-	): Promise<GitRemote<RichRemoteProvider> | undefined> {
+	): Promise<GitRemote<RemoteProvider | RichRemoteProvider> | undefined> {
 		if (remotesOrRepoPath == null) return undefined;
 
 		let remotes;
+		let repoPath;
 		if (Array.isArray(remotesOrRepoPath)) {
 			if (remotesOrRepoPath.length === 0) return undefined;
 
 			remotes = remotesOrRepoPath;
-			remotesOrRepoPath = remotesOrRepoPath[0].repoPath;
+			repoPath = remotesOrRepoPath[0].repoPath;
+		} else {
+			repoPath = remotesOrRepoPath;
 		}
 
-		if (typeof remotesOrRepoPath === 'string') {
-			remotesOrRepoPath = this.getAbsoluteUri(remotesOrRepoPath);
+		if (typeof repoPath === 'string') {
+			repoPath = this.getAbsoluteUri(repoPath);
 		}
 
-		const cacheKey = asRepoComparisonKey(remotesOrRepoPath);
-
-		let richRemote = this._richRemotesCache.get(cacheKey);
-		if (richRemote != null) return richRemote;
-		if (richRemote === null && !options?.includeDisconnected) return undefined;
-
-		if (options?.includeDisconnected) {
-			richRemote = this._richRemotesCache.get(`disconnected|${cacheKey}`);
-			if (richRemote !== undefined) return richRemote ?? undefined;
-		}
+		const cacheKey = asRepoComparisonKey(repoPath);
+		let remote = this._bestRemotesCache.get(cacheKey);
+		if (remote !== undefined) return remote ?? undefined;
 
-		remotes = (remotes ?? (await this.getRemotesWithProviders(remotesOrRepoPath))).filter(
+		remotes = (remotes ?? (await this.getRemotesWithProviders(repoPath))).filter(
 			(
 				r: GitRemote<RemoteProvider | RichRemoteProvider | undefined>,
 			): r is GitRemote<RemoteProvider | RichRemoteProvider> => r.provider != null,
@@ -1722,7 +1717,6 @@ export class GitProviderService implements Disposable {
 
 		if (remotes.length === 0) return undefined;
 
-		let remote;
 		if (remotes.length === 1) {
 			remote = remotes[0];
 		} else {
@@ -1757,18 +1751,85 @@ export class GitProviderService implements Disposable {
 			remote = bestRemote ?? null;
 		}
 
+		this._bestRemotesCache.set(cacheKey, remote);
+
+		return remote ?? undefined;
+	}
+
+	async getBestRemoteWithRichProvider(
+		repoPath: string | Uri | undefined,
+		options?: { includeDisconnected?: boolean },
+	): Promise<GitRemote<RichRemoteProvider> | undefined>;
+	async getBestRemoteWithRichProvider(
+		remotes: GitRemote[],
+		options?: { includeDisconnected?: boolean },
+	): Promise<GitRemote<RichRemoteProvider> | undefined>;
+	@gate<GitProviderService['getBestRemoteWithRichProvider']>(
+		(remotesOrRepoPath, options) =>
+			`${
+				remotesOrRepoPath == null || typeof remotesOrRepoPath === 'string'
+					? remotesOrRepoPath
+					: remotesOrRepoPath instanceof Uri
+					? remotesOrRepoPath.toString()
+					: `${remotesOrRepoPath.length}:${remotesOrRepoPath[0]?.repoPath ?? ''}`
+			}|${options?.includeDisconnected ?? false}`,
+	)
+	@log<GitProviderService['getBestRemoteWithRichProvider']>({
+		args: {
+			0: remotesOrRepoPath =>
+				Array.isArray(remotesOrRepoPath) ? remotesOrRepoPath.map(r => r.name).join(',') : remotesOrRepoPath,
+		},
+	})
+	async getBestRemoteWithRichProvider(
+		remotesOrRepoPath: GitRemote[] | string | Uri | undefined,
+		options?: { includeDisconnected?: boolean },
+	): Promise<GitRemote<RichRemoteProvider> | undefined> {
+		if (remotesOrRepoPath == null) return undefined;
+
+		let remotes;
+		let repoPath;
+		if (Array.isArray(remotesOrRepoPath)) {
+			if (remotesOrRepoPath.length === 0) return undefined;
+
+			remotes = remotesOrRepoPath;
+			repoPath = remotesOrRepoPath[0].repoPath;
+		} else {
+			repoPath = remotesOrRepoPath;
+		}
+
+		if (typeof repoPath === 'string') {
+			repoPath = this.getAbsoluteUri(repoPath);
+		}
+
+		const cacheKey = asRepoComparisonKey(repoPath);
+
+		let richRemote = this._bestRemotesCache.get(`rich+connected|${cacheKey}`);
+		if (richRemote != null) return richRemote;
+		if (richRemote === null && !options?.includeDisconnected) return undefined;
+
+		if (options?.includeDisconnected) {
+			richRemote = this._bestRemotesCache.get(`rich|${cacheKey}`);
+			if (richRemote !== undefined) return richRemote ?? undefined;
+		}
+
+		const remote = await (remotes != null
+			? this.getBestRemoteWithProvider(remotes)
+			: this.getBestRemoteWithProvider(repoPath));
+
 		if (!remote?.hasRichProvider()) {
-			this._richRemotesCache.set(cacheKey, null);
+			this._bestRemotesCache.set(`rich|${cacheKey}`, null);
+			this._bestRemotesCache.set(`rich+connected|${cacheKey}`, null);
 			return undefined;
 		}
 
 		const { provider } = remote;
 		const connected = provider.maybeConnected ?? (await provider.isConnected());
 		if (connected) {
-			this._richRemotesCache.set(cacheKey, remote);
+			this._bestRemotesCache.set(`rich|${cacheKey}`, remote);
+			this._bestRemotesCache.set(`rich+connected|${cacheKey}`, remote);
 		} else {
-			this._richRemotesCache.set(cacheKey, null);
-			this._richRemotesCache.set(`disconnected|${cacheKey}`, remote);
+			this._bestRemotesCache.set(`rich|${cacheKey}`, remote);
+			this._bestRemotesCache.set(`rich+connected|${cacheKey}`, null);
 
 			if (!options?.includeDisconnected) return undefined;
 		}

src/git/models/commit.ts

@@ -392,7 +392,7 @@ export class GitCommit implements GitRevisionReference {
 	async getAssociatedPullRequest(options?: { timeout?: number }): Promise<PullRequest | undefined> {
 		if (this._pullRequest == null) {
 			async function getCore(this: GitCommit): Promise<PullRequest | undefined> {
-				const remote = await this.container.git.getRichRemoteProvider(this.repoPath);
+				const remote = await this.container.git.getBestRemoteWithRichProvider(this.repoPath);
 				if (remote?.provider == null) return undefined;
 
 				return this.container.git.getPullRequestForCommit(this.ref, remote, options);

src/git/models/repository.ts

@@ -588,7 +588,7 @@ export class Repository implements Disposable {
 	}
 
 	async getRichRemote(connectedOnly: boolean = false): Promise<GitRemote<RichRemoteProvider> | undefined> {
-		return this.container.git.getRichRemoteProvider(await this.getRemotes(), {
+		return this.container.git.getBestRemoteWithRichProvider(await this.getRemotes(), {
 			includeDisconnected: !connectedOnly,
 		});
 	}

src/hovers/hovers.ts

@@ -297,7 +297,7 @@ export namespace Hovers {
 			return undefined;
 		}
 
-		const remote = await Container.instance.git.getRichRemoteProvider(remotes);
+		const remote = await Container.instance.git.getBestRemoteWithRichProvider(remotes);
 		if (remote?.provider == null) {
 			Logger.debug(cc, `completed ${GlyphChars.Dot} ${getDurationMilliseconds(start)} ms`);
 
@@ -372,7 +372,9 @@ export namespace Hovers {
 			return undefined;
 		}
 
-		const remote = await Container.instance.git.getRichRemoteProvider(remotes, { includeDisconnected: true });
+		const remote = await Container.instance.git.getBestRemoteWithRichProvider(remotes, {
+			includeDisconnected: true,
+		});
 		if (remote?.provider == null) {
 			Logger.debug(cc, `completed ${GlyphChars.Dot} ${getDurationMilliseconds(start)} ms`);
 

src/statusbar/statusBarController.ts

@@ -341,7 +341,7 @@ export class StatusBarController implements Disposable {
 		commit: GitCommit,
 		{ timeout }: { timeout?: number } = {},
 	): Promise<PullRequest | PromiseCancelledError<Promise<PullRequest | undefined>> | undefined> {
-		const remote = await this.container.git.getRichRemoteProvider(commit.repoPath);
+		const remote = await this.container.git.getBestRemoteWithRichProvider(commit.repoPath);
 		if (remote?.provider == null) return undefined;
 
 		const { provider } = remote;

src/views/nodes/autolinkedItemNode.ts

@@ -1,4 +1,5 @@
-import { MarkdownString, TreeItem, TreeItemCollapsibleState } from 'vscode';
+import { MarkdownString, ThemeIcon, TreeItem, TreeItemCollapsibleState } from 'vscode';
+import { Autolink } from '../../annotations/autolinks';
 import { GitUri } from '../../git/gitUri';
 import { GitFile, IssueOrPullRequest, IssueOrPullRequestType } from '../../git/models';
 import { fromNow } from '../../system/date';
@@ -19,17 +20,17 @@ export class AutolinkedItemNode extends ViewNode {
 		view: ViewsWithCommits,
 		parent: ViewNode,
 		public readonly repoPath: string,
-		public readonly issue: IssueOrPullRequest,
+		public readonly item: Autolink | IssueOrPullRequest,
 	) {
 		super(GitUri.fromRepoPath(repoPath), view, parent);
 	}
 
 	override toClipboard(): string {
-		return this.issue.url;
+		return this.item.url;
 	}
 
 	override get id(): string {
-		return `${this.parent!.id!}:item(${this.issue.id})`;
+		return `${this.parent!.id!}:item(${this.item.id})`;
 	}
 
 	getChildren(): ViewNode[] {
@@ -37,24 +38,55 @@ export class AutolinkedItemNode extends ViewNode {
 	}
 
 	getTreeItem(): TreeItem {
-		const relativeTime = fromNow(this.issue.closedDate ?? this.issue.date);
+		if (!isIssueOrPullRequest(this.item)) {
+			const { provider } = this.item;
 
-		const item = new TreeItem(`${this.issue.id}: ${this.issue.title}`, TreeItemCollapsibleState.None);
+			const item = new TreeItem(
+				`${provider?.name ? `${provider.name} ` : ''}${this.item.prefix}${this.item.id}`,
+				TreeItemCollapsibleState.None,
+			);
+			item.iconPath = new ThemeIcon(
+				this.item.type == null
+					? 'link'
+					: this.item.type === IssueOrPullRequestType.PullRequest
+					? 'git-pull-request'
+					: 'issues',
+			);
+			item.contextValue = ContextValues.AutolinkedItem;
+			item.tooltip = new MarkdownString(
+				`${
+					this.item.description
+						? `Autolink: ${this.item.description}`
+						: `${
+								this.item.type == null
+									? 'Autolink'
+									: this.item.type === IssueOrPullRequestType.PullRequest
+									? 'Autolinked Pull Request'
+									: 'Autolinked Issue'
+						  }: **${this.item.prefix}${this.item.id}**`
+				} \\\n[${this.item.url}](${this.item.url}${this.item.title != null ? ` "${this.item.title}"` : ''})`,
+			);
+			return item;
+		}
+
+		const relativeTime = fromNow(this.item.closedDate ?? this.item.date);
+
+		const item = new TreeItem(`${this.item.id}: ${this.item.title}`, TreeItemCollapsibleState.None);
 		item.description = relativeTime;
-		item.iconPath = IssueOrPullRequest.getThemeIcon(this.issue);
+		item.iconPath = IssueOrPullRequest.getThemeIcon(this.item);
 		item.contextValue =
-			this.issue.type === IssueOrPullRequestType.PullRequest
+			this.item.type === IssueOrPullRequestType.PullRequest
 				? ContextValues.PullRequest
 				: ContextValues.AutolinkedIssue;
 
 		const linkTitle = ` "Open ${
-			this.issue.type === IssueOrPullRequestType.PullRequest ? 'Pull Request' : 'Issue'
-		} \\#${this.issue.id} on ${this.issue.provider.name}"`;
+			this.item.type === IssueOrPullRequestType.PullRequest ? 'Pull Request' : 'Issue'
+		} \\#${this.item.id} on ${this.item.provider.name}"`;
 		const tooltip = new MarkdownString(
-			`${IssueOrPullRequest.getMarkdownIcon(this.issue)} [**${this.issue.title.trim()}**](${
-				this.issue.url
-			}${linkTitle}) \\\n[#${this.issue.id}](${this.issue.url}${linkTitle}) was ${
-				this.issue.closed ? 'closed' : 'opened'
+			`${IssueOrPullRequest.getMarkdownIcon(this.item)} [**${this.item.title.trim()}**](${
+				this.item.url
+			}${linkTitle}) \\\n[#${this.item.id}](${this.item.url}${linkTitle}) was ${
+				this.item.closed ? 'closed' : 'opened'
 			} ${relativeTime}`,
 			true,
 		);
@@ -66,3 +98,7 @@ export class AutolinkedItemNode extends ViewNode {
 		return item;
 	}
 }
+
+function isIssueOrPullRequest(item: Autolink | IssueOrPullRequest): item is IssueOrPullRequest {
+	return 'closed' in item && typeof item.closed === 'boolean';
+}

src/views/nodes/commitNode.ts

@@ -131,7 +131,7 @@ export class CommitNode extends ViewRefNode
 
 	private async getTooltip() {
 		const remotes = await this.view.container.git.getRemotesWithProviders(this.commit.repoPath, { sort: true });
-		const remote = await this.view.container.git.getRichRemoteProvider(remotes);
+		const remote = await this.view.container.git.getBestRemoteWithRichProvider(remotes);
 
 		if (this.commit.message == null) {
 			await this.commit.ensureFullDetails();

src/views/nodes/fileRevisionAsCommitNode.ts

@@ -198,7 +198,7 @@ export class FileRevisionAsCommitNode extends ViewRefFileNode
 
 	private async getTooltip() {
 		const remotes = await this.view.container.git.getRemotesWithProviders(this.commit.repoPath);
-		const remote = await this.view.container.git.getRichRemoteProvider(remotes);
+		const remote = await this.view.container.git.getBestRemoteWithRichProvider(remotes);
 
 		if (this.commit.message == null) {
 			await this.commit.ensureFullDetails();

src/views/nodes/rebaseStatusNode.ts

@@ -189,7 +189,7 @@ export class RebaseCommitNode extends ViewRefNode
 
 	private async getTooltip() {
 		const remotes = await this.view.container.git.getRemotesWithProviders(this.commit.repoPath);
-		const remote = await this.view.container.git.getRichRemoteProvider(remotes);
+		const remote = await this.view.container.git.getBestRemoteWithRichProvider(remotes);
 
 		if (this.commit.message == null) {
 			await this.commit.ensureFullDetails();

src/views/nodes/resultsCommitsNode.ts

@@ -1,11 +1,11 @@
 import { TreeItem, TreeItemCollapsibleState } from 'vscode';
 import { GitUri } from '../../git/gitUri';
-import { GitLog } from '../../git/models';
+import type { GitLog } from '../../git/models';
 import { gate } from '../../system/decorators/gate';
 import { debug } from '../../system/decorators/log';
 import { map } from '../../system/iterable';
 import { cancellable, PromiseCancelledError } from '../../system/promise';
-import { ViewsWithCommits } from '../viewBase';
+import type { ViewsWithCommits } from '../viewBase';
 import { AutolinkedItemsNode } from './autolinkedItemsNode';
 import { CommitNode } from './commitNode';
 import { LoadMoreNode } from './common';
@@ -73,7 +73,7 @@ export class ResultsCommitsNode
 
 		const [getBranchAndTagTips, provider] = await Promise.all([
 			this.view.container.git.getBranchesAndTagsTipsFn(this.uri.repoPath),
-			this.view.container.git.getRichRemoteProvider(this.repoPath),
+			this.view.container.git.getBestRemoteWithProvider(this.repoPath),
 		]);
 
 		const children = [];