Skip to content

Commit d10f867

Browse files
committed
Improves "best" remote detection
Improves autolink targets -- refs: #2425
1 parent 22f5838 commit d10f867

File tree

11 files changed

+271
-19
lines changed

11 files changed

+271
-19
lines changed

src/annotations/autolinks.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,11 @@ export class Autolinks implements Disposable {
257257
}
258258

259259
if (remotes != null && remotes.length !== 0) {
260+
remotes = [...remotes].sort((a, b) => {
261+
const aConnected = a.provider?.maybeConnected;
262+
const bConnected = b.provider?.maybeConnected;
263+
return aConnected !== bConnected ? (aConnected ? -1 : bConnected ? 1 : 0) : 0;
264+
});
260265
for (const r of remotes) {
261266
if (r.provider == null) continue;
262267

src/git/gitProviderService.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2166,7 +2166,16 @@ export class GitProviderService implements Disposable {
21662166
}
21672167

21682168
// Don't choose a remote unless its weighted above
2169-
const matchedWeight = weightedRemotes.get(r.name) ?? -1;
2169+
let matchedWeight = weightedRemotes.get(r.name) ?? -1;
2170+
2171+
const p = r.provider;
2172+
if (p.hasRichIntegration() && p.maybeConnected) {
2173+
const m = await p.getRepositoryMetadata();
2174+
if (m?.isFork === false) {
2175+
matchedWeight += 101;
2176+
}
2177+
}
2178+
21702179
if (matchedWeight > weight) {
21712180
bestRemote = r;
21722181
weight = matchedWeight;
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import type { RemoteProviderReference } from './remoteProvider';
2+
3+
export interface RepositoryMetadata {
4+
provider: RemoteProviderReference;
5+
owner: string;
6+
name: string;
7+
isFork: boolean;
8+
parent?: {
9+
owner: string;
10+
name: string;
11+
};
12+
}

src/git/remotes/github.ts

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import type { IssueOrPullRequest, SearchedIssue } from '../models/issue';
1818
import type { PullRequest, PullRequestState, SearchedPullRequest } from '../models/pullRequest';
1919
import { isSha } from '../models/reference';
2020
import type { Repository } from '../models/repository';
21+
import type { RepositoryMetadata } from '../models/repositoryMetadata';
2122
import { ensurePaidPlan, RichRemoteProvider } from './richRemoteProvider';
2223

2324
const autolinkFullIssuesRegex = /\b(?<repo>[^/\s]+\/[^/\s]+)#(?<num>[0-9]+)\b(?!]\()/g;
@@ -259,7 +260,7 @@ export class GitHubRemote extends RichRemoteProvider {
259260
return `${this.encodeUrl(`${this.baseUrl}?path=${fileName}`)}${line}`;
260261
}
261262

262-
protected async getProviderAccountForCommit(
263+
protected override async getProviderAccountForCommit(
263264
{ accessToken }: AuthenticationSession,
264265
ref: string,
265266
options?: {
@@ -273,7 +274,7 @@ export class GitHubRemote extends RichRemoteProvider {
273274
});
274275
}
275276

276-
protected async getProviderAccountForEmail(
277+
protected override async getProviderAccountForEmail(
277278
{ accessToken }: AuthenticationSession,
278279
email: string,
279280
options?: {
@@ -287,7 +288,7 @@ export class GitHubRemote extends RichRemoteProvider {
287288
});
288289
}
289290

290-
protected async getProviderDefaultBranch({
291+
protected override async getProviderDefaultBranch({
291292
accessToken,
292293
}: AuthenticationSession): Promise<DefaultBranch | undefined> {
293294
const [owner, repo] = this.splitPath();
@@ -296,7 +297,7 @@ export class GitHubRemote extends RichRemoteProvider {
296297
});
297298
}
298299

299-
protected async getProviderIssueOrPullRequest(
300+
protected override async getProviderIssueOrPullRequest(
300301
{ accessToken }: AuthenticationSession,
301302
id: string,
302303
): Promise<IssueOrPullRequest | undefined> {
@@ -306,7 +307,7 @@ export class GitHubRemote extends RichRemoteProvider {
306307
});
307308
}
308309

309-
protected async getProviderPullRequestForBranch(
310+
protected override async getProviderPullRequestForBranch(
310311
{ accessToken }: AuthenticationSession,
311312
branch: string,
312313
options?: {
@@ -326,7 +327,7 @@ export class GitHubRemote extends RichRemoteProvider {
326327
});
327328
}
328329

329-
protected async getProviderPullRequestForCommit(
330+
protected override async getProviderPullRequestForCommit(
330331
{ accessToken }: AuthenticationSession,
331332
ref: string,
332333
): Promise<PullRequest | undefined> {
@@ -336,7 +337,16 @@ export class GitHubRemote extends RichRemoteProvider {
336337
});
337338
}
338339

339-
protected async searchProviderMyPullRequests({
340+
protected override async getProviderRepositoryMetadata({
341+
accessToken,
342+
}: AuthenticationSession): Promise<RepositoryMetadata | undefined> {
343+
const [owner, repo] = this.splitPath();
344+
return (await this.container.github)?.getRepositoryMetadata(this, accessToken, owner, repo, {
345+
baseUrl: this.apiBaseUrl,
346+
});
347+
}
348+
349+
protected override async searchProviderMyPullRequests({
340350
accessToken,
341351
}: AuthenticationSession): Promise<SearchedPullRequest[] | undefined> {
342352
return (await this.container.github)?.searchMyPullRequests(this, accessToken, {
@@ -345,7 +355,7 @@ export class GitHubRemote extends RichRemoteProvider {
345355
});
346356
}
347357

348-
protected async searchProviderMyIssues({
358+
protected override async searchProviderMyIssues({
349359
accessToken,
350360
}: AuthenticationSession): Promise<SearchedIssue[] | undefined> {
351361
return (await this.container.github)?.searchMyIssues(this, accessToken, {

src/git/remotes/gitlab.ts

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import type { IssueOrPullRequest, SearchedIssue } from '../models/issue';
1818
import type { PullRequest, PullRequestState, SearchedPullRequest } from '../models/pullRequest';
1919
import { isSha } from '../models/reference';
2020
import type { Repository } from '../models/repository';
21+
import type { RepositoryMetadata } from '../models/repositoryMetadata';
2122
import { ensurePaidPlan, RichRemoteProvider } from './richRemoteProvider';
2223

2324
const autolinkFullIssuesRegex = /\b(?<repo>[^/\s]+\/[^/\s]+)#(?<num>[0-9]+)\b(?!]\()/g;
@@ -290,7 +291,7 @@ export class GitLabRemote extends RichRemoteProvider {
290291
return `${this.encodeUrl(`${this.baseUrl}?path=${fileName}`)}${line}`;
291292
}
292293

293-
protected async getProviderAccountForCommit(
294+
protected override async getProviderAccountForCommit(
294295
{ accessToken }: AuthenticationSession,
295296
ref: string,
296297
options?: {
@@ -304,7 +305,7 @@ export class GitLabRemote extends RichRemoteProvider {
304305
});
305306
}
306307

307-
protected async getProviderAccountForEmail(
308+
protected override async getProviderAccountForEmail(
308309
{ accessToken }: AuthenticationSession,
309310
email: string,
310311
options?: {
@@ -318,7 +319,7 @@ export class GitLabRemote extends RichRemoteProvider {
318319
});
319320
}
320321

321-
protected async getProviderDefaultBranch({
322+
protected override async getProviderDefaultBranch({
322323
accessToken,
323324
}: AuthenticationSession): Promise<DefaultBranch | undefined> {
324325
const [owner, repo] = this.splitPath();
@@ -327,7 +328,7 @@ export class GitLabRemote extends RichRemoteProvider {
327328
});
328329
}
329330

330-
protected async getProviderIssueOrPullRequest(
331+
protected override async getProviderIssueOrPullRequest(
331332
{ accessToken }: AuthenticationSession,
332333
id: string,
333334
): Promise<IssueOrPullRequest | undefined> {
@@ -337,7 +338,7 @@ export class GitLabRemote extends RichRemoteProvider {
337338
});
338339
}
339340

340-
protected async getProviderPullRequestForBranch(
341+
protected override async getProviderPullRequestForBranch(
341342
{ accessToken }: AuthenticationSession,
342343
branch: string,
343344
options?: {
@@ -357,7 +358,7 @@ export class GitLabRemote extends RichRemoteProvider {
357358
});
358359
}
359360

360-
protected async getProviderPullRequestForCommit(
361+
protected override async getProviderPullRequestForCommit(
361362
{ accessToken }: AuthenticationSession,
362363
ref: string,
363364
): Promise<PullRequest | undefined> {
@@ -367,13 +368,24 @@ export class GitLabRemote extends RichRemoteProvider {
367368
});
368369
}
369370

370-
protected async searchProviderMyPullRequests(
371+
protected override async getProviderRepositoryMetadata({
372+
accessToken,
373+
}: AuthenticationSession): Promise<RepositoryMetadata | undefined> {
374+
const [owner, repo] = this.splitPath();
375+
return (await this.container.gitlab)?.getRepositoryMetadata(this, accessToken, owner, repo, {
376+
baseUrl: this.apiBaseUrl,
377+
});
378+
}
379+
380+
protected override async searchProviderMyPullRequests(
371381
_session: AuthenticationSession,
372382
): Promise<SearchedPullRequest[] | undefined> {
373383
return Promise.resolve(undefined);
374384
}
375385

376-
protected async searchProviderMyIssues(_session: AuthenticationSession): Promise<SearchedIssue[] | undefined> {
386+
protected override async searchProviderMyIssues(
387+
_session: AuthenticationSession,
388+
): Promise<SearchedIssue[] | undefined> {
377389
return Promise.resolve(undefined);
378390
}
379391
}

src/git/remotes/remoteProvider.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ export abstract class RemoteProvider implements RemoteProviderReference {
5959
return this.type === 'rich';
6060
}
6161

62+
get maybeConnected(): boolean | undefined {
63+
return false;
64+
}
65+
6266
abstract getLocalInfoFromRemoteUri(
6367
repository: Repository,
6468
uri: Uri,

src/git/remotes/richRemoteProvider.ts

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import type { Account } from '../models/author';
1717
import type { DefaultBranch } from '../models/defaultBranch';
1818
import type { IssueOrPullRequest, SearchedIssue } from '../models/issue';
1919
import type { PullRequest, PullRequestState, SearchedPullRequest } from '../models/pullRequest';
20+
import type { RepositoryMetadata } from '../models/repositoryMetadata';
2021
import { RemoteProvider } from './remoteProvider';
2122

2223
// TODO@eamodio revisit how once authenticated, all remotes are always connected, even after a restart
@@ -73,7 +74,7 @@ export abstract class RichRemoteProvider extends RemoteProvider {
7374
return `connected:${this.key}`;
7475
}
7576

76-
get maybeConnected(): boolean | undefined {
77+
override get maybeConnected(): boolean | undefined {
7778
return this._session === undefined ? undefined : this._session !== null;
7879
}
7980

@@ -145,6 +146,7 @@ export abstract class RichRemoteProvider extends RemoteProvider {
145146
}
146147

147148
this.resetRequestExceptionCount();
149+
this._repoMetadata = undefined;
148150
this._prsByCommit.clear();
149151
this._session = null;
150152

@@ -286,6 +288,37 @@ export abstract class RichRemoteProvider extends RemoteProvider {
286288
accessToken,
287289
}: AuthenticationSession): Promise<DefaultBranch | undefined>;
288290

291+
private _repoMetadata: RepositoryMetadata | undefined;
292+
293+
@gate()
294+
@debug()
295+
async getRepositoryMetadata(): Promise<RepositoryMetadata | undefined> {
296+
if (this._repoMetadata != null) return this._repoMetadata;
297+
298+
const scope = getLogScope();
299+
300+
const connected = this.maybeConnected ?? (await this.isConnected());
301+
if (!connected) return undefined;
302+
303+
try {
304+
const metadata = await this.getProviderRepositoryMetadata(this._session!);
305+
this._repoMetadata = metadata;
306+
this.resetRequestExceptionCount();
307+
return metadata;
308+
} catch (ex) {
309+
Logger.error(ex, scope);
310+
311+
if (ex instanceof AuthenticationError || ex instanceof ProviderRequestClientError) {
312+
this.trackRequestException();
313+
}
314+
return undefined;
315+
}
316+
}
317+
318+
protected abstract getProviderRepositoryMetadata({
319+
accessToken,
320+
}: AuthenticationSession): Promise<RepositoryMetadata | undefined>;
321+
289322
@gate()
290323
@debug()
291324
async searchMyPullRequests(): Promise<SearchedPullRequest[] | undefined> {

src/plus/github/github.ts

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import type { DefaultBranch } from '../../git/models/defaultBranch';
2323
import type { IssueOrPullRequest, SearchedIssue } from '../../git/models/issue';
2424
import type { PullRequest, SearchedPullRequest } from '../../git/models/pullRequest';
2525
import { isSha } from '../../git/models/reference';
26+
import type { RepositoryMetadata } from '../../git/models/repositoryMetadata';
2627
import type { GitUser } from '../../git/models/user';
2728
import { getGitHubNoReplyAddressParts } from '../../git/remotes/github';
2829
import type { RichRemoteProvider } from '../../git/remotes/richRemoteProvider';
@@ -736,6 +737,93 @@ export class GitHubApi implements Disposable {
736737
}
737738
}
738739

740+
@debug<GitHubApi['getRepositoryMetadata']>({ args: { 0: p => p.name, 1: '<token>' } })
741+
async getRepositoryMetadata(
742+
provider: RichRemoteProvider,
743+
token: string,
744+
owner: string,
745+
repo: string,
746+
options?: {
747+
baseUrl?: string;
748+
},
749+
): Promise<RepositoryMetadata | undefined> {
750+
const scope = getLogScope();
751+
752+
interface QueryResult {
753+
repository:
754+
| {
755+
owner: {
756+
login: string;
757+
};
758+
name: string;
759+
parent:
760+
| {
761+
owner: {
762+
login: string;
763+
};
764+
name: string;
765+
}
766+
| null
767+
| undefined;
768+
}
769+
| null
770+
| undefined;
771+
}
772+
773+
try {
774+
const query = `query getRepositoryMetadata(
775+
$owner: String!
776+
$repo: String!
777+
) {
778+
repository(name: $repo, owner: $owner) {
779+
owner {
780+
login
781+
}
782+
name
783+
parent {
784+
owner {
785+
login
786+
}
787+
name
788+
}
789+
}
790+
}`;
791+
792+
const rsp = await this.graphql<QueryResult>(
793+
provider,
794+
token,
795+
query,
796+
{
797+
...options,
798+
owner: owner,
799+
repo: repo,
800+
},
801+
scope,
802+
);
803+
804+
const r = rsp?.repository ?? undefined;
805+
if (r == null) return undefined;
806+
807+
return {
808+
provider: provider,
809+
owner: r.owner.login,
810+
name: r.name,
811+
isFork: r.parent != null,
812+
parent:
813+
r.parent != null
814+
? {
815+
owner: r.parent.owner.login,
816+
name: r.parent.name,
817+
}
818+
: undefined,
819+
};
820+
} catch (ex) {
821+
if (ex instanceof ProviderRequestNotFoundError) return undefined;
822+
823+
throw this.handleException(ex, provider, scope);
824+
}
825+
}
826+
739827
@debug<GitHubApi['getBlame']>({ args: { 0: '<token>' } })
740828
async getBlame(token: string, owner: string, repo: string, ref: string, path: string): Promise<GitHubBlame> {
741829
const scope = getLogScope();

0 commit comments

Comments
 (0)