From 2067332776884045f9b54d3b2fe293719af8f669 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?And=C5=BEej=20Korovacki?= Date: Sat, 22 Apr 2023 21:48:05 +0300 Subject: [PATCH 1/3] Check git setting to fetch before checkout in checkoutExistingPullRequestBranch --- src/github/pullRequestGitHelper.ts | 50 +++++++++++++++++++++++------- 1 file changed, 39 insertions(+), 11 deletions(-) diff --git a/src/github/pullRequestGitHelper.ts b/src/github/pullRequestGitHelper.ts index 5b67cb4557..e4322131cd 100644 --- a/src/github/pullRequestGitHelper.ts +++ b/src/github/pullRequestGitHelper.ts @@ -12,6 +12,7 @@ import { GitErrorCodes } from '../api/api1'; import Logger from '../common/logger'; import { Protocol } from '../common/protocol'; import { parseRepositoryRemotes, Remote } from '../common/remote'; +import { GIT, PULL_BEFORE_CHECKOUT } from '../common/settingKeys'; import { IResolvedPullRequestModel, PullRequestModel } from './pullRequestModel'; const PullRequestRemoteMetadataKey = 'github-pr-remote'; @@ -31,7 +32,7 @@ export class PullRequestGitHelper { repository: Repository, pullRequest: PullRequestModel & IResolvedPullRequestModel, remoteName: string | undefined, - progress: vscode.Progress<{ message?: string; increment?: number }> + progress: vscode.Progress<{ message?: string; increment?: number }>, ) { // the branch is from a fork const localBranchName = await PullRequestGitHelper.calculateUniqueBranchNameForPR(repository, pullRequest); @@ -42,7 +43,12 @@ export class PullRequestGitHelper { `Branch ${localBranchName} is from a fork. Create a remote first.`, PullRequestGitHelper.ID, ); - progress.report({ message: vscode.l10n.t('Creating git remote for {0}', `${pullRequest.remote.owner}/${pullRequest.remote.repositoryName}`) }); + progress.report({ + message: vscode.l10n.t( + 'Creating git remote for {0}', + `${pullRequest.remote.owner}/${pullRequest.remote.repositoryName}`, + ), + }); remoteName = await PullRequestGitHelper.createRemote( repository, pullRequest.remote, @@ -69,7 +75,7 @@ export class PullRequestGitHelper { repository: Repository, remotes: Remote[], pullRequest: PullRequestModel, - progress: vscode.Progress<{ message?: string; increment?: number }> + progress: vscode.Progress<{ message?: string; increment?: number }>, ): Promise { if (!pullRequest.validatePullRequestModel('Checkout pull request failed')) { return; @@ -78,7 +84,12 @@ export class PullRequestGitHelper { const remote = PullRequestGitHelper.getHeadRemoteForPullRequest(remotes, pullRequest); const isFork = pullRequest.head.repositoryCloneUrl.owner !== pullRequest.base.repositoryCloneUrl.owner; if (!remote || isFork) { - return PullRequestGitHelper.checkoutFromFork(repository, pullRequest, remote && remote.remoteName, progress); + return PullRequestGitHelper.checkoutFromFork( + repository, + pullRequest, + remote && remote.remoteName, + progress, + ); } const branchName = pullRequest.head.ref; @@ -89,7 +100,10 @@ export class PullRequestGitHelper { branch = await repository.getBranch(branchName); // Make sure we aren't already on this branch if (repository.state.HEAD?.name === branch.name) { - Logger.appendLine(`Tried to checkout ${branchName}, but branch is already checked out.`, PullRequestGitHelper.ID); + Logger.appendLine( + `Tried to checkout ${branchName}, but branch is already checked out.`, + PullRequestGitHelper.ID, + ); return; } Logger.debug(`Checkout ${branchName}`, PullRequestGitHelper.ID); @@ -166,7 +180,11 @@ export class PullRequestGitHelper { } } - static async checkoutExistingPullRequestBranch(repository: Repository, pullRequest: PullRequestModel, progress: vscode.Progress<{ message?: string; increment?: number }>) { + static async checkoutExistingPullRequestBranch( + repository: Repository, + pullRequest: PullRequestModel, + progress: vscode.Progress<{ message?: string; increment?: number }>, + ) { const key = PullRequestGitHelper.buildPullRequestMetadata(pullRequest); const configs = await repository.getConfigs(); @@ -188,14 +206,20 @@ export class PullRequestGitHelper { const branchName = branchInfos[0].branch!; progress.report({ message: vscode.l10n.t('Checking out branch {0}', branchName) }); await repository.checkout(branchName); - const remote = readConfig(`branch.${branchName}.remote`); - const ref = readConfig(`branch.${branchName}.merge`); - progress.report({ message: vscode.l10n.t('Fetching branch {0}', branchName) }); - await repository.fetch(remote, ref); + + // respect the git setting to fetch before checkout + if (vscode.workspace.getConfiguration(GIT).get(PULL_BEFORE_CHECKOUT, false)) { + const remote = readConfig(`branch.${branchName}.remote`); + const ref = readConfig(`branch.${branchName}.merge`); + progress.report({ message: vscode.l10n.t('Fetching branch {0}', branchName) }); + await repository.fetch(remote, ref); + } + const branchStatus = await repository.getBranch(branchInfos[0].branch!); if (branchStatus.upstream === undefined) { return false; } + if (branchStatus.behind !== undefined && branchStatus.behind > 0 && branchStatus.ahead === 0) { Logger.debug(`Pull from upstream`, PullRequestGitHelper.ID); progress.report({ message: vscode.l10n.t('Pulling branch {0}', branchName) }); @@ -392,7 +416,11 @@ export class PullRequestGitHelper { pullRequest: PullRequestModel & IResolvedPullRequestModel, ): Remote | undefined { return remotes.find( - remote => remote.gitProtocol && (remote.gitProtocol.owner.toLowerCase() === pullRequest.head.repositoryCloneUrl.owner.toLowerCase()) && (remote.gitProtocol.repositoryName.toLowerCase() === pullRequest.head.repositoryCloneUrl.repositoryName.toLowerCase()) + remote => + remote.gitProtocol && + remote.gitProtocol.owner.toLowerCase() === pullRequest.head.repositoryCloneUrl.owner.toLowerCase() && + remote.gitProtocol.repositoryName.toLowerCase() === + pullRequest.head.repositoryCloneUrl.repositoryName.toLowerCase(), ); } From 2a6f9cba306a3c3691438545de4b2dfbfb1e8bd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?And=C5=BEej=20Korovacki?= Date: Wed, 10 May 2023 13:57:44 +0300 Subject: [PATCH 2/3] formatting changes reverted --- src/github/pullRequestGitHelper.ts | 35 ++++++------------------------ 1 file changed, 7 insertions(+), 28 deletions(-) diff --git a/src/github/pullRequestGitHelper.ts b/src/github/pullRequestGitHelper.ts index e4322131cd..d596a7ec92 100644 --- a/src/github/pullRequestGitHelper.ts +++ b/src/github/pullRequestGitHelper.ts @@ -32,7 +32,7 @@ export class PullRequestGitHelper { repository: Repository, pullRequest: PullRequestModel & IResolvedPullRequestModel, remoteName: string | undefined, - progress: vscode.Progress<{ message?: string; increment?: number }>, + progress: vscode.Progress<{ message?: string; increment?: number }> ) { // the branch is from a fork const localBranchName = await PullRequestGitHelper.calculateUniqueBranchNameForPR(repository, pullRequest); @@ -43,12 +43,7 @@ export class PullRequestGitHelper { `Branch ${localBranchName} is from a fork. Create a remote first.`, PullRequestGitHelper.ID, ); - progress.report({ - message: vscode.l10n.t( - 'Creating git remote for {0}', - `${pullRequest.remote.owner}/${pullRequest.remote.repositoryName}`, - ), - }); + progress.report({ message: vscode.l10n.t('Creating git remote for {0}', `${pullRequest.remote.owner}/${pullRequest.remote.repositoryName}`) }); remoteName = await PullRequestGitHelper.createRemote( repository, pullRequest.remote, @@ -75,7 +70,7 @@ export class PullRequestGitHelper { repository: Repository, remotes: Remote[], pullRequest: PullRequestModel, - progress: vscode.Progress<{ message?: string; increment?: number }>, + progress: vscode.Progress<{ message?: string; increment?: number }> ): Promise { if (!pullRequest.validatePullRequestModel('Checkout pull request failed')) { return; @@ -84,12 +79,7 @@ export class PullRequestGitHelper { const remote = PullRequestGitHelper.getHeadRemoteForPullRequest(remotes, pullRequest); const isFork = pullRequest.head.repositoryCloneUrl.owner !== pullRequest.base.repositoryCloneUrl.owner; if (!remote || isFork) { - return PullRequestGitHelper.checkoutFromFork( - repository, - pullRequest, - remote && remote.remoteName, - progress, - ); + return PullRequestGitHelper.checkoutFromFork(repository, pullRequest, remote && remote.remoteName, progress); } const branchName = pullRequest.head.ref; @@ -100,10 +90,7 @@ export class PullRequestGitHelper { branch = await repository.getBranch(branchName); // Make sure we aren't already on this branch if (repository.state.HEAD?.name === branch.name) { - Logger.appendLine( - `Tried to checkout ${branchName}, but branch is already checked out.`, - PullRequestGitHelper.ID, - ); + Logger.appendLine(`Tried to checkout ${branchName}, but branch is already checked out.`, PullRequestGitHelper.ID); return; } Logger.debug(`Checkout ${branchName}`, PullRequestGitHelper.ID); @@ -180,11 +167,7 @@ export class PullRequestGitHelper { } } - static async checkoutExistingPullRequestBranch( - repository: Repository, - pullRequest: PullRequestModel, - progress: vscode.Progress<{ message?: string; increment?: number }>, - ) { + static async checkoutExistingPullRequestBranch(repository: Repository, pullRequest: PullRequestModel, progress: vscode.Progress<{ message?: string; increment?: number }>) { const key = PullRequestGitHelper.buildPullRequestMetadata(pullRequest); const configs = await repository.getConfigs(); @@ -416,11 +399,7 @@ export class PullRequestGitHelper { pullRequest: PullRequestModel & IResolvedPullRequestModel, ): Remote | undefined { return remotes.find( - remote => - remote.gitProtocol && - remote.gitProtocol.owner.toLowerCase() === pullRequest.head.repositoryCloneUrl.owner.toLowerCase() && - remote.gitProtocol.repositoryName.toLowerCase() === - pullRequest.head.repositoryCloneUrl.repositoryName.toLowerCase(), + remote => remote.gitProtocol && (remote.gitProtocol.owner.toLowerCase() === pullRequest.head.repositoryCloneUrl.owner.toLowerCase()) && (remote.gitProtocol.repositoryName.toLowerCase() === pullRequest.head.repositoryCloneUrl.repositoryName.toLowerCase()) ); } From cacca49bdb6d6d71ad450b96f8145a047ab60cac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?And=C5=BEej=20Korovacki?= Date: Thu, 11 May 2023 22:46:21 +0300 Subject: [PATCH 3/3] Added new setting to control if PR has to be pulled before checkout --- package.json | 5 +++++ package.nls.json | 1 + src/common/settingKeys.ts | 1 + src/github/pullRequestGitHelper.ts | 4 ++-- 4 files changed, 9 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 843937d737..716a182edf 100644 --- a/package.json +++ b/package.json @@ -347,6 +347,11 @@ "description": "%githubPullRequests.setAutoMerge.description%", "default": false }, + "githubPullRequests.pullPullRequestBranchBeforeCheckout": { + "type": "boolean", + "description": "%githubPullRequests.pullPullRequestBranchBeforeCheckout.description%", + "default": true + }, "githubIssues.ignoreMilestones": { "type": "array", "default": [], diff --git a/package.nls.json b/package.nls.json index b1359b0989..adca36dfa7 100644 --- a/package.nls.json +++ b/package.nls.json @@ -63,6 +63,7 @@ "githubPullRequests.defaultCommentType.single": "Submits the comment as a single comment that will be immediately visible to other users", "githubPullRequests.defaultCommentType.review": "Submits the comment as a review comment that will be visible to other users once the review is submitted", "githubPullRequests.setAutoMerge.description": "Checks the \"Auto-merge\" checkbox in the \"Create Pull Request\" view.", + "githubPullRequests.pullPullRequestBranchBeforeCheckout.description": "Pulls pull request before checkout", "githubIssues.ignoreMilestones.description": "An array of milestones titles to never show issues from.", "githubIssues.createIssueTriggers.description": "Strings that will cause the 'Create issue from comment' code action to show.", "githubIssues.createIssueTriggers.items": "String that enables the 'Create issue from comment' code action. Should not contain whitespace.", diff --git a/src/common/settingKeys.ts b/src/common/settingKeys.ts index bed8634e94..43f2893ddd 100644 --- a/src/common/settingKeys.ts +++ b/src/common/settingKeys.ts @@ -27,6 +27,7 @@ export const DEFAULT_DELETION_METHOD = 'defaultDeletionMethod'; export const SELECT_LOCAL_BRANCH = 'selectLocalBranch'; export const SELECT_REMOTE = 'selectRemote'; export const REMOTES = 'remotes'; +export const PULL_PR_BRANCH_BEFORE_CHECKOUT = 'pullPullRequestBranchBeforeCheckout'; export const ISSUES_SETTINGS_NAMESPACE = 'githubIssues'; export const ASSIGN_WHEN_WORKING = 'assignWhenWorking'; diff --git a/src/github/pullRequestGitHelper.ts b/src/github/pullRequestGitHelper.ts index d596a7ec92..0994487259 100644 --- a/src/github/pullRequestGitHelper.ts +++ b/src/github/pullRequestGitHelper.ts @@ -12,7 +12,7 @@ import { GitErrorCodes } from '../api/api1'; import Logger from '../common/logger'; import { Protocol } from '../common/protocol'; import { parseRepositoryRemotes, Remote } from '../common/remote'; -import { GIT, PULL_BEFORE_CHECKOUT } from '../common/settingKeys'; +import { PR_SETTINGS_NAMESPACE, PULL_PR_BRANCH_BEFORE_CHECKOUT } from '../common/settingKeys'; import { IResolvedPullRequestModel, PullRequestModel } from './pullRequestModel'; const PullRequestRemoteMetadataKey = 'github-pr-remote'; @@ -191,7 +191,7 @@ export class PullRequestGitHelper { await repository.checkout(branchName); // respect the git setting to fetch before checkout - if (vscode.workspace.getConfiguration(GIT).get(PULL_BEFORE_CHECKOUT, false)) { + if (vscode.workspace.getConfiguration(PR_SETTINGS_NAMESPACE).get(PULL_PR_BRANCH_BEFORE_CHECKOUT, true)) { const remote = readConfig(`branch.${branchName}.remote`); const ref = readConfig(`branch.${branchName}.merge`); progress.report({ message: vscode.l10n.t('Fetching branch {0}', branchName) });