From 3b291e841a5a621a01af8ed7f3e810a3969bc142 Mon Sep 17 00:00:00 2001 From: Alex Ross Date: Mon, 15 Jan 2024 16:40:08 +0100 Subject: [PATCH] Allow a PR to be brought up to date with main and resolve conflicts Fixes #1562, #200 --- package.json | 3 +- src/@types/git.d.ts | 2 + .../vscode.proposed.tabInputTextMerge.d.ts | 25 +++ src/api/api.d.ts | 2 + src/github/conflictGuide.ts | 150 ++++++++++++++++++ src/github/graphql.ts | 1 + src/github/interface.ts | 1 + src/github/pullRequestOverview.ts | 36 +++++ src/github/queries.gql | 1 + src/github/queriesExtra.gql | 1 + src/github/queriesLimited.gql | 1 + src/github/utils.ts | 8 +- src/github/views.ts | 1 + .../builders/graphql/pullRequestBuilder.ts | 1 + src/test/mocks/mockRepository.ts | 8 + webviews/common/common.css | 5 + webviews/common/context.tsx | 4 + webviews/components/merge.tsx | 28 +++- .../editorWebview/test/builder/pullRequest.ts | 1 + 19 files changed, 271 insertions(+), 8 deletions(-) create mode 100644 src/@types/vscode.proposed.tabInputTextMerge.d.ts create mode 100644 src/github/conflictGuide.ts diff --git a/package.json b/package.json index ab5f38ed37..a26069a9a8 100644 --- a/package.json +++ b/package.json @@ -23,12 +23,13 @@ "shareProvider", "quickDiffProvider", "readonlyMessage", + "tabInputTextMerge", "treeViewMarkdownMessage" ], "version": "0.78.1", "publisher": "GitHub", "engines": { - "vscode": "^1.85.0" + "vscode": "^1.86.0" }, "categories": [ "Other" diff --git a/src/@types/git.d.ts b/src/@types/git.d.ts index b3d6fbceaf..e225cc2982 100644 --- a/src/@types/git.d.ts +++ b/src/@types/git.d.ts @@ -233,6 +233,8 @@ export interface Repository { log(options?: LogOptions): Promise; commit(message: string, opts?: CommitOptions): Promise; + merge(ref: string): Promise; + mergeAbort(): Promise; } export interface RemoteSource { diff --git a/src/@types/vscode.proposed.tabInputTextMerge.d.ts b/src/@types/vscode.proposed.tabInputTextMerge.d.ts new file mode 100644 index 0000000000..9f90ea88dd --- /dev/null +++ b/src/@types/vscode.proposed.tabInputTextMerge.d.ts @@ -0,0 +1,25 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +// https://github.com/microsoft/vscode/issues/153213 + +declare module 'vscode' { + + export class TabInputTextMerge { + + readonly base: Uri; + readonly input1: Uri; + readonly input2: Uri; + readonly result: Uri; + + constructor(base: Uri, input1: Uri, input2: Uri, result: Uri); + } + + export interface Tab { + + readonly input: TabInputText | TabInputTextDiff | TabInputTextMerge | TabInputCustom | TabInputWebview | TabInputNotebook | TabInputNotebookDiff | TabInputTerminal | unknown; + + } +} diff --git a/src/api/api.d.ts b/src/api/api.d.ts index 98c8fb8d2e..4b4947cba1 100644 --- a/src/api/api.d.ts +++ b/src/api/api.d.ts @@ -205,6 +205,8 @@ export interface Repository { commit(message: string, opts?: CommitOptions): Promise; add(paths: string[]): Promise; + merge(ref: string): Promise; + mergeAbort(): Promise; } /** diff --git a/src/github/conflictGuide.ts b/src/github/conflictGuide.ts new file mode 100644 index 0000000000..4033ff4106 --- /dev/null +++ b/src/github/conflictGuide.ts @@ -0,0 +1,150 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as vscode from 'vscode'; +import { Change, Repository } from '../api/api'; +import { commands } from '../common/executeCommands'; +import { asPromise, dispose } from '../common/utils'; + +export class ConflictGuide implements vscode.Disposable { + private _progress: vscode.Progress<{ message?: string; increment?: number }> | undefined; + private readonly _startingConflictsCount: number; + private readonly _oneProgressIncrement: number; + private _lastReportedRemainingCount: number; + private _disposables: vscode.Disposable[] = []; + private _finishedCommit: vscode.EventEmitter = new vscode.EventEmitter(); + private _finishedConflicts: vscode.EventEmitter = new vscode.EventEmitter(); + private _message: string; + + constructor(private readonly _repository: Repository, private readonly _upstream: string, private readonly _into: string) { + this._startingConflictsCount = this.remainingConflicts.length; + this._lastReportedRemainingCount = this._startingConflictsCount; + this._oneProgressIncrement = 100 / this._startingConflictsCount; + this._repository.inputBox.value = this._message = `Merge branch '${this._upstream}' into ${this._into}`; + this._watchForRemainingConflictsChange(); + } + + private _watchForRemainingConflictsChange() { + this._disposables.push(vscode.window.tabGroups.onDidChangeTabs(async (e) => { + if (e.closed.length > 0) { + await this._repository.status(); + this._reportProgress(); + } + })); + } + + private _reportProgress() { + const remainingCount = this.remainingConflicts.length; + if (this._progress) { + const increment = (this._lastReportedRemainingCount - remainingCount) * this._oneProgressIncrement; + this._progress.report({ message: vscode.l10n.t('Use the Source Control view to resolve conflicts, {0} of {0} remaining', remainingCount, this._startingConflictsCount), increment }); + this._lastReportedRemainingCount = remainingCount; + } + if (remainingCount === 0) { + this._finishedConflicts.fire(true); + this.commit(); + } + } + + private async commitFromNotification(): Promise { + const commit = vscode.l10n.t('Commit'); + const cancel = vscode.l10n.t('Abort Merge'); + const result = await vscode.window.showInformationMessage(vscode.l10n.t('All conflicts resolved. Commit and push the resolution to continue.'), commit, cancel); + if (result === commit) { + await this._repository.commit(this._message); + this._repository.inputBox.value = ''; + await this._repository.push(); + return true; + } else { + await this.abort(); + return false; + } + } + + private async commit() { + let localDisposable: vscode.Disposable | undefined; + const scmCommit = new Promise(resolve => { + const startingCommit = this._repository.state.HEAD?.commit; + localDisposable = this._repository.state.onDidChange(() => { + if (this._repository.state.HEAD?.commit !== startingCommit && this._repository.state.indexChanges.length === 0 && this._repository.state.mergeChanges.length === 0) { + resolve(true); + } + }); + this._disposables.push(localDisposable); + }); + + const notificationCommit = this.commitFromNotification(); + + const result = await Promise.race([scmCommit, notificationCommit]); + localDisposable?.dispose(); + this._finishedCommit.fire(result); + } + + get remainingConflicts(): Change[] { + return this._repository.state.mergeChanges; + } + + private async closeMergeEditors(): Promise { + for (const group of vscode.window.tabGroups.all) { + for (const tab of group.tabs) { + if (tab.input instanceof vscode.TabInputTextMerge) { + vscode.window.tabGroups.close(tab); + } + } + } + } + + private async abort(): Promise { + this._repository.inputBox.value = ''; + // set up an event to listen for when we are all out of merge changes before closing the merge editors. + // Just waiting for the merge doesn't cut it + // Even with this, we still need to wait 1 second, and then it still might say there are conflicts. Why is this? + const disposable = this._repository.state.onDidChange(async () => { + if (this._repository.state.mergeChanges.length === 0) { + await new Promise(resolve => setTimeout(resolve, 1000)); + this.closeMergeEditors(); + disposable.dispose(); + } + }); + await this._repository.mergeAbort(); + this._finishedCommit.fire(false); + } + + private async first(progress: vscode.Progress<{ message?: string; increment?: number }>, cancellationToken: vscode.CancellationToken): Promise { + this._progress = progress; + if (this.remainingConflicts.length === 0) { + return; + } + await commands.focusView('workbench.scm'); + this._reportProgress(); + const change = this.remainingConflicts[0]; + this._disposables.push(cancellationToken.onCancellationRequested(() => this.abort())); + await commands.executeCommand('git.openMergeEditor', change.uri); + } + + public static async begin(repository: Repository, upstream: string, into: string): Promise { + const wizard = new ConflictGuide(repository, upstream, into); + if (wizard.remainingConflicts.length === 0) { + return undefined; + } + vscode.window.withProgress({ location: vscode.ProgressLocation.Notification, cancellable: true }, async (progress, token) => { + wizard.first(progress, token); + return wizard.finishedConflicts(); + }); + return wizard; + } + + private finishedConflicts(): Promise { + return asPromise(this._finishedConflicts.event); + } + + public finished(): Promise { + return asPromise(this._finishedCommit.event); + } + + dispose() { + dispose(this._disposables); + } +} \ No newline at end of file diff --git a/src/github/graphql.ts b/src/github/graphql.ts index 1bf331f743..6c88f9959c 100644 --- a/src/github/graphql.ts +++ b/src/github/graphql.ts @@ -603,6 +603,7 @@ export interface PullRequest { }; viewerCanEnableAutoMerge: boolean; viewerCanDisableAutoMerge: boolean; + viewerCanUpdateBranch: boolean; isDraft?: boolean; suggestedReviewers: SuggestedReviewerResponse[]; projectItems?: { diff --git a/src/github/interface.ts b/src/github/interface.ts index f4acc433b2..68866e37d1 100644 --- a/src/github/interface.ts +++ b/src/github/interface.ts @@ -179,6 +179,7 @@ export interface PullRequest extends Issue { merged?: boolean; mergeable?: PullRequestMergeability; mergeQueueEntry?: MergeQueueEntry | null; + canUpdateBranch: boolean; autoMerge?: boolean; autoMergeMethod?: MergeMethod; allowAutoMerge?: boolean; diff --git a/src/github/pullRequestOverview.ts b/src/github/pullRequestOverview.ts index 2e5b2ca2c9..3f9a710461 100644 --- a/src/github/pullRequestOverview.ts +++ b/src/github/pullRequestOverview.ts @@ -5,6 +5,7 @@ 'use strict'; import * as vscode from 'vscode'; +import { GitErrorCodes } from '../api/api1'; import { onDidUpdatePR, openPullRequestOnGitHub } from '../commands'; import { IComment } from '../common/comment'; import Logger from '../common/logger'; @@ -12,6 +13,7 @@ import { DEFAULT_MERGE_METHOD, PR_SETTINGS_NAMESPACE } from '../common/settingKe import { ReviewEvent as CommonReviewEvent } from '../common/timelineEvent'; import { asPromise, dispose, formatError } from '../common/utils'; import { IRequestMessage, PULL_REQUEST_OVERVIEW_VIEW_TYPE } from '../common/webview'; +import { ConflictGuide } from './conflictGuide'; import { FolderRepositoryManager } from './folderRepositoryManager'; import { GithubItemStateEnum, @@ -23,6 +25,7 @@ import { ITeam, MergeMethod, MergeMethodsAvailability, + PullRequestMergeability, reviewerId, ReviewEvent, ReviewState, @@ -262,6 +265,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel): Promise { + if (this._folderRepositoryManager.repository.state.workingTreeChanges.length > 0 || this._folderRepositoryManager.repository.state.indexChanges.length > 0) { + await vscode.window.showErrorMessage(vscode.l10n.t('The pull request branch cannot be updated when the there changed files in the working tree or index. Stash or commit all change and then try again.'), { modal: true }); + return this._replyMessage(message, {}); + } + const qualifiedUpstream = `${this._item.remote.remoteName}/${this._item.base.ref}`; + await vscode.window.withProgress({ location: vscode.ProgressLocation.Notification }, async (progress) => { + progress.report({ message: vscode.l10n.t('Fetching branch') }); + await this._folderRepositoryManager.repository.fetch({ ref: this._item.base.ref, remote: this._item.remote.remoteName }); + progress.report({ message: vscode.l10n.t('Merging branch') }); + try { + await this._folderRepositoryManager.repository.merge(qualifiedUpstream); + } catch (e) { + if (e.gitErrorCode !== GitErrorCodes.Conflict) { + throw e; + } + } + }); + + if (this._item.item.mergeable === PullRequestMergeability.Conflict) { + const wizard = await ConflictGuide.begin(this._folderRepositoryManager.repository, this._item.base.ref, this._folderRepositoryManager.repository.state.HEAD!.name!); + await wizard?.finished(); + wizard?.dispose(); + } else { + await this._folderRepositoryManager.repository.push(); + } + + this._replyMessage(message, {}); + } + protected editCommentPromise(comment: IComment, text: string): Promise { return this._item.editReviewComment(comment, text); } diff --git a/src/github/queries.gql b/src/github/queries.gql index 812fc908ac..cb28d49b9b 100644 --- a/src/github/queries.gql +++ b/src/github/queries.gql @@ -82,6 +82,7 @@ fragment PullRequestFragment on PullRequest { } viewerCanEnableAutoMerge viewerCanDisableAutoMerge + viewerCanUpdateBranch id databaseId isDraft diff --git a/src/github/queriesExtra.gql b/src/github/queriesExtra.gql index 9e8df8d511..fb2ce5f1cb 100644 --- a/src/github/queriesExtra.gql +++ b/src/github/queriesExtra.gql @@ -82,6 +82,7 @@ fragment PullRequestFragment on PullRequest { } viewerCanEnableAutoMerge viewerCanDisableAutoMerge + viewerCanUpdateBranch id databaseId isDraft diff --git a/src/github/queriesLimited.gql b/src/github/queriesLimited.gql index df60096c47..a3e3b6593c 100644 --- a/src/github/queriesLimited.gql +++ b/src/github/queriesLimited.gql @@ -73,6 +73,7 @@ fragment PullRequestFragment on PullRequest { } viewerCanEnableAutoMerge viewerCanDisableAutoMerge + viewerCanUpdateBranch id databaseId isDraft diff --git a/src/github/utils.ts b/src/github/utils.ts index f5349ad276..d69b1e8fb1 100644 --- a/src/github/utils.ts +++ b/src/github/utils.ts @@ -317,6 +317,7 @@ export function convertRESTPullRequestToRawPullRequest( : undefined, createdAt: created_at, updatedAt: updated_at, + canUpdateBranch: false, head: head.repo ? convertRESTHeadToIGitHubRef(head as OctokitCommon.PullsListResponseItemHead) : undefined, base: convertRESTHeadToIGitHubRef(base), labels: labels.map(l => ({ name: '', color: '', ...l })), @@ -339,7 +340,7 @@ export function convertRESTPullRequestToRawPullRequest( export function convertRESTIssueToRawPullRequest( pullRequest: OctokitCommon.IssuesCreateResponseData, githubRepository: GitHubRepository, -): PullRequest { +): Issue { const { number, body, @@ -355,7 +356,7 @@ export function convertRESTIssueToRawPullRequest( id, } = pullRequest; - const item: PullRequest = { + const item: Issue = { id, graphNodeId: node_id, number, @@ -373,9 +374,7 @@ export function convertRESTIssueToRawPullRequest( labels: labels.map(l => typeof l === 'string' ? { name: l, color: '' } : { name: l.name ?? '', color: l.color ?? '', description: l.description ?? undefined }, ), - suggestedReviewers: [], // suggested reviewers only available through GraphQL API, projectItems: [], // projects only available through GraphQL API - commits: [], // commits only available through GraphQL API }; return item; @@ -702,6 +701,7 @@ export function parseGraphQLPullRequest( autoMerge: !!graphQLPullRequest.autoMergeRequest, autoMergeMethod: parseMergeMethod(graphQLPullRequest.autoMergeRequest?.mergeMethod), allowAutoMerge: graphQLPullRequest.viewerCanEnableAutoMerge || graphQLPullRequest.viewerCanDisableAutoMerge, + canUpdateBranch: graphQLPullRequest.viewerCanUpdateBranch, labels: graphQLPullRequest.labels.nodes, isDraft: graphQLPullRequest.isDraft, suggestedReviewers: parseSuggestedReviewers(graphQLPullRequest.suggestedReviewers), diff --git a/src/github/views.ts b/src/github/views.ts index 373f1fd365..0c13364f82 100644 --- a/src/github/views.ts +++ b/src/github/views.ts @@ -62,6 +62,7 @@ export interface PullRequest { pendingReviewType?: ReviewType; status: PullRequestChecks | null; reviewRequirement: PullRequestReviewRequirement | null; + canUpdateBranch: boolean; mergeable: PullRequestMergeability; defaultMergeMethod: MergeMethod; mergeMethodsAvailability: MergeMethodsAvailability; diff --git a/src/test/builders/graphql/pullRequestBuilder.ts b/src/test/builders/graphql/pullRequestBuilder.ts index 1483f95fce..3e9b158cf0 100644 --- a/src/test/builders/graphql/pullRequestBuilder.ts +++ b/src/test/builders/graphql/pullRequestBuilder.ts @@ -94,6 +94,7 @@ export const PullRequestBuilder = createBuilderClass()({ suggestedReviewers: { default: [] }, viewerCanEnableAutoMerge: { default: false }, viewerCanDisableAutoMerge: { default: false }, + viewerCanUpdateBranch: { default: false }, commits: createLink()({ nodes: { default: [ diff --git a/src/test/mocks/mockRepository.ts b/src/test/mocks/mockRepository.ts index 8d719ac72f..60c051c515 100644 --- a/src/test/mocks/mockRepository.ts +++ b/src/test/mocks/mockRepository.ts @@ -319,4 +319,12 @@ export class MockRepository implements Repository { expectPush(remoteName?: string, branchName?: string, setUpstream?: boolean) { this._expectedPushes.push({ remoteName, branchName, setUpstream }); } + + merge(ref: string): Promise { + return Promise.reject(new Error(`Unexpected merge(${ref})`)); + } + + mergeAbort(): Promise { + return Promise.reject(new Error(`Unexpected mergeAbort`)); + } } diff --git a/webviews/common/common.css b/webviews/common/common.css index 5f10120f57..f97be30552 100644 --- a/webviews/common/common.css +++ b/webviews/common/common.css @@ -242,6 +242,11 @@ body img.avatar { color: var(--vscode-editor-foreground); } +.status-item button { + margin-left: auto; + margin-right: 0; +} + .automerge-section { display: flex; } diff --git a/webviews/common/context.tsx b/webviews/common/context.tsx index 71d01b1f31..7a821f1df0 100644 --- a/webviews/common/context.tsx +++ b/webviews/common/context.tsx @@ -180,6 +180,10 @@ export class PRContext { this.updatePR(state); } + public updateBranch = async () => { + return this.postMessage({ command: 'pr.update-branch' }); + } + public dequeue = async () => { const isDequeued = await this.postMessage({ command: 'pr.dequeue' }); const state = this.pr; diff --git a/webviews/components/merge.tsx b/webviews/components/merge.tsx index 0b4d1eee95..be37bfa64c 100644 --- a/webviews/components/merge.tsx +++ b/webviews/components/merge.tsx @@ -199,7 +199,8 @@ export const MergeStatusAndActions = ({ pr, isSimple }: { pr: PullRequest; isSim return (
- + +
); @@ -207,21 +208,26 @@ export const MergeStatusAndActions = ({ pr, isSimple }: { pr: PullRequest; isSim export default StatusChecksSection; -export const MergeStatus = ({ mergeable, isSimple }: { mergeable: PullRequestMergeability; isSimple: boolean }) => { +export const MergeStatus = ({ mergeable, isSimple, isCurrentlyCheckedOut, canUpdateBranch }: { mergeable: PullRequestMergeability; isSimple: boolean; isCurrentlyCheckedOut: boolean, canUpdateBranch: boolean }) => { + const { updateBranch } = useContext(PullRequestContext); + let icon: JSX.Element | null = pendingIcon; let summary: string = 'Checking if this branch can be merged...'; + let action: string | null = null; if (mergeable === PullRequestMergeability.Mergeable) { icon = checkIcon; summary = 'This branch has no conflicts with the base branch.'; } else if (mergeable === PullRequestMergeability.Conflict) { icon = closeIcon; summary = 'This branch has conflicts that must be resolved.'; + action = 'Resolve conflicts'; } else if (mergeable === PullRequestMergeability.NotMergeable) { icon = closeIcon; summary = 'Branch protection policy must be fulfilled before merging.'; } else if (mergeable === PullRequestMergeability.Behind) { - icon = alertIcon; + icon = closeIcon; summary = 'This branch is out-of-date with the base branch.'; + action = 'Update with merge commit'; } if (isSimple) { @@ -233,10 +239,26 @@ export const MergeStatus = ({ mergeable, isSimple }: { mergeable: PullRequestMer

{summary}

+ {(action && !isSimple && canUpdateBranch && isCurrentlyCheckedOut) ? : null} ); }; +export const OfferToUpdate = ({ mergeable, isSimple, isCurrentlyCheckedOut, canUpdateBranch }: { mergeable: PullRequestMergeability; isSimple: boolean; isCurrentlyCheckedOut: boolean, canUpdateBranch: boolean }) => { + const { updateBranch } = useContext(PullRequestContext); + if (!canUpdateBranch || !isCurrentlyCheckedOut || isSimple || mergeable === PullRequestMergeability.Behind || mergeable === PullRequestMergeability.Conflict || mergeable === PullRequestMergeability.Unknown) { + return null; + } + return ( +
+ {alertIcon} +

This branch is out-of-date with the base branch.

+ +
+ ); + +}; + export const ReadyForReview = ({ isSimple }: { isSimple: boolean }) => { const [isBusy, setBusy] = useState(false); const { readyForReview, updatePR } = useContext(PullRequestContext); diff --git a/webviews/editorWebview/test/builder/pullRequest.ts b/webviews/editorWebview/test/builder/pullRequest.ts index aae8212411..19ac7fa75d 100644 --- a/webviews/editorWebview/test/builder/pullRequest.ts +++ b/webviews/editorWebview/test/builder/pullRequest.ts @@ -42,6 +42,7 @@ export const PullRequestBuilder = createBuilderClass()({ allowAutoMerge: { default: false }, mergeQueueEntry: { default: undefined }, mergeQueueMethod: { default: undefined }, + canUpdateBranch: { default: false }, reviewers: { default: [] }, isDraft: { default: false }, isIssue: { default: false },