Skip to content

Commit

Permalink
Improves getting the associated pr with a branch
Browse files Browse the repository at this point in the history 
Removes `getPullRequestForBranch` from GitProviderService as it doesn't belong there
  • Loading branch information
@eamodio
eamodio committed Sep 26, 2023
1 parent 5b2e46a commit 6de04e3
Show file tree
Hide file tree
Showing 9 changed files with 91 additions and 149 deletions.
54 changes: 20 additions & 34 deletions src/commands/openAssociatedPullRequestOnRemote.ts
View file
Expand Up @@ -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;
Expand All @@ -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);
}
}
8 changes: 6 additions & 2 deletions src/commands/openPullRequestOnRemote.ts
View file
@@ -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';
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 3 additions & 1 deletion src/env/node/git/localGitProvider.ts
View file
Expand Up @@ -1753,6 +1753,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
]);

branch = new GitBranch(
this.container,
repoPath,
rebaseStatus?.incoming.name ?? name,
false,
Expand Down Expand Up @@ -1801,6 +1802,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
]);

current = new GitBranch(
this.container,
repoPath!,
rebaseStatus?.incoming.name ?? name,
false,
Expand All @@ -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!);

Expand Down
77 changes: 16 additions & 61 deletions src/git/gitProviderService.ts
View file
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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];
}
Expand All @@ -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);
Expand All @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
63 changes: 19 additions & 44 deletions src/git/models/branch.ts
View file
@@ -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';
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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();
}

Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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?: {
Expand All @@ -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);
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/git/parsers/branchParser.ts
View file
@@ -1,3 +1,4 @@
import type { Container } from '../../container';
import { debug } from '../../system/decorators/log';
import { GitBranch } from '../models/branch';

Expand All @@ -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;
Expand Down Expand Up @@ -57,6 +58,7 @@ export class GitBranchParser {

branches.push(
new GitBranch(
container,
repoPath,
name,
remote,
Expand Down

0 comments on commit 6de04e3

Please sign in to comment.