From 25093fc14795d7753b8697613cf413b1b017086d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pouya=20Kary=20=E2=9C=A8?= Date: Tue, 16 Jan 2024 15:40:45 +0330 Subject: [PATCH 1/6] added foreground colors to the package.json --- package.json | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/package.json b/package.json index ab5f38ed3..b95980780 100644 --- a/package.json +++ b/package.json @@ -2624,6 +2624,16 @@ }, "description": "The color used for indicating that an issue is open." }, + { + "id": "issues.open.foreground", + "defaults": { + "dark": "#E1FFD9", + "light": "#E1FFD9", + "highContrast": "editor.background", + "highContrastLight": "editor.background" + }, + "description": "The foreground color of badges used for indicating that an issue is open." + }, { "id": "issues.closed", "defaults": { @@ -2634,6 +2644,16 @@ }, "description": "The color used for indicating that an issue is closed." }, + { + "id": "issues.closed.foreground", + "defaults": { + "dark": "#FFD9D9", + "light": "#FFD9D9", + "highContrast": "editor.background", + "highContrastLight": "editor.background" + }, + "description": "The foreground color of badges used for indicating that an issue is closed." + }, { "id": "pullRequests.merged", "defaults": { @@ -2644,6 +2664,16 @@ }, "description": "The color used for indicating that a pull request is merged." }, + { + "id": "pullRequests.merged.foreground", + "defaults": { + "dark": "#FFD9FE", + "light": "#FFD9FE", + "highContrast": "editor.foreground", + "highContrastLight": "editor.foreground" + }, + "description": "The badge foreground color used for indicating that a pull request is merged." + }, { "id": "pullRequests.draft", "defaults": { @@ -2654,6 +2684,16 @@ }, "description": "The color used for indicating that a pull request is a draft." }, + { + "id": "pullRequests.draft.foreground", + "defaults": { + "dark": "#EEEEEE", + "light": "#EEEEEE", + "highContrast": "editor.foreground", + "highContrastLight": "editor.foreground" + }, + "description": "The badge foreground color used for indicating that a pull request is draft." + }, { "id": "pullRequests.open", "defaults": { @@ -2664,6 +2704,16 @@ }, "description": "The color used for indicating that a pull request is open." }, + { + "id": "pullRequests.open.foreground", + "defaults": { + "dark": "issues.open.foreground", + "light": "issues.open.foreground", + "highContrast": "editor.foreground", + "highContrastLight": "editor.foreground" + }, + "description": "The foreground color of badges used for indicating that a pull request is open." + }, { "id": "pullRequests.closed", "defaults": { @@ -2674,6 +2724,16 @@ }, "description": "The color used for indicating that a pull request is closed." }, + { + "id": "pullRequests.closed.foreground", + "defaults": { + "dark": "issues.closed.foreground", + "light": "issues.closed.foreground", + "highContrast": "editor.foreground", + "highContrastLight": "editor.foreground" + }, + "description": "The foreground color of badges used for indicating that a pull request is closed." + }, { "id": "pullRequests.notification", "defaults": { From 923c07e7cd91314067bb189a48dcd017274f7cb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pouya=20Kary=20=E2=9C=A8?= Date: Tue, 16 Jan 2024 15:41:01 +0330 Subject: [PATCH 2/6] added foreground colors to status badges of pr requests --- webviews/editorWebview/index.css | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/webviews/editorWebview/index.css b/webviews/editorWebview/index.css index 44037dc6c..5f8056433 100644 --- a/webviews/editorWebview/index.css +++ b/webviews/editorWebview/index.css @@ -488,18 +488,22 @@ body button .icon { .status-badge-merged { background-color: var(--vscode-pullRequests-merged); + color: var(--vscode-pullRequests-merged-foreground) } .status-badge-open { background-color: var(--vscode-pullRequests-open); + color: var(--vscode-pullRequests-open-foreground) } .status-badge-closed { background-color: var(--vscode-pullRequests-closed); + color: var(--vscode-pullRequests-closed-foreground) } .status-badge-draft { background-color: var(--vscode-pullRequests-draft); + color: var(--vscode-pullRequests-draft-foreground) } .section { From 1428cd8bda1f8cb6111d9ff6fb47624c2956c014 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pouya=20Kary=20=E2=9C=A8?= Date: Tue, 16 Jan 2024 15:45:16 +0330 Subject: [PATCH 3/6] fixed the colors --- package.json | 40 ++++++++++------------------------------ 1 file changed, 10 insertions(+), 30 deletions(-) diff --git a/package.json b/package.json index b95980780..7c7d93c4a 100644 --- a/package.json +++ b/package.json @@ -2624,16 +2624,6 @@ }, "description": "The color used for indicating that an issue is open." }, - { - "id": "issues.open.foreground", - "defaults": { - "dark": "#E1FFD9", - "light": "#E1FFD9", - "highContrast": "editor.background", - "highContrastLight": "editor.background" - }, - "description": "The foreground color of badges used for indicating that an issue is open." - }, { "id": "issues.closed", "defaults": { @@ -2644,16 +2634,6 @@ }, "description": "The color used for indicating that an issue is closed." }, - { - "id": "issues.closed.foreground", - "defaults": { - "dark": "#FFD9D9", - "light": "#FFD9D9", - "highContrast": "editor.background", - "highContrastLight": "editor.background" - }, - "description": "The foreground color of badges used for indicating that an issue is closed." - }, { "id": "pullRequests.merged", "defaults": { @@ -2707,12 +2687,12 @@ { "id": "pullRequests.open.foreground", "defaults": { - "dark": "issues.open.foreground", - "light": "issues.open.foreground", - "highContrast": "editor.foreground", - "highContrastLight": "editor.foreground" + "dark": "#E1FFD9", + "light": "#E1FFD9", + "highContrast": "editor.background", + "highContrastLight": "editor.background" }, - "description": "The foreground color of badges used for indicating that a pull request is open." + "description": "The foreground color of badges used for indicating that an pull request is open." }, { "id": "pullRequests.closed", @@ -2727,12 +2707,12 @@ { "id": "pullRequests.closed.foreground", "defaults": { - "dark": "issues.closed.foreground", - "light": "issues.closed.foreground", - "highContrast": "editor.foreground", - "highContrastLight": "editor.foreground" + "dark": "#FFD9D9", + "light": "#FFD9D9", + "highContrast": "editor.background", + "highContrastLight": "editor.background" }, - "description": "The foreground color of badges used for indicating that a pull request is closed." + "description": "The foreground color of badges used for indicating that an pull requests is closed." }, { "id": "pullRequests.notification", From d90a5c271c464c265f1e65d64f46d01153d74859 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pouya=20Kary=20=E2=9C=A8?= Date: Thu, 18 Jan 2024 18:00:43 +0330 Subject: [PATCH 4/6] Applied the changed requested by @alexr00 --- package.json | 40 ++++++++++++++++---------------- webviews/editorWebview/index.css | 8 +++---- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/package.json b/package.json index 7c7d93c4a..15835c4ae 100644 --- a/package.json +++ b/package.json @@ -2645,14 +2645,14 @@ "description": "The color used for indicating that a pull request is merged." }, { - "id": "pullRequests.merged.foreground", + "id": "pullRequests.statusMergedForeground", "defaults": { - "dark": "#FFD9FE", - "light": "#FFD9FE", + "dark": "pullRequests.statusOpenForeground", + "light": "pullRequests.statusOpenForeground", "highContrast": "editor.foreground", "highContrastLight": "editor.foreground" }, - "description": "The badge foreground color used for indicating that a pull request is merged." + "description": "The foreground color used for the status badge indicating that a pull request is merged." }, { "id": "pullRequests.draft", @@ -2665,14 +2665,14 @@ "description": "The color used for indicating that a pull request is a draft." }, { - "id": "pullRequests.draft.foreground", + "id": "pullRequests.statusDraftForeground", "defaults": { - "dark": "#EEEEEE", - "light": "#EEEEEE", + "dark": "pullRequests.statusOpenForeground", + "light": "pullRequests.statusOpenForeground", "highContrast": "editor.foreground", "highContrastLight": "editor.foreground" }, - "description": "The badge foreground color used for indicating that a pull request is draft." + "description": "The foreground color used for the status badge indicating that a pull request is draft." }, { "id": "pullRequests.open", @@ -2685,14 +2685,14 @@ "description": "The color used for indicating that a pull request is open." }, { - "id": "pullRequests.open.foreground", + "id": "pullRequests.statusOpenForeground", "defaults": { - "dark": "#E1FFD9", - "light": "#E1FFD9", - "highContrast": "editor.background", - "highContrastLight": "editor.background" + "dark": "#000000", + "light": "#ffffff", + "highContrast": "editor.foreground", + "highContrastLight": "editor.foreground" }, - "description": "The foreground color of badges used for indicating that an pull request is open." + "description": "The foreground color used for the status badge indicating that a pull request is open." }, { "id": "pullRequests.closed", @@ -2705,14 +2705,14 @@ "description": "The color used for indicating that a pull request is closed." }, { - "id": "pullRequests.closed.foreground", + "id": "pullRequests.statusClosedForeground", "defaults": { - "dark": "#FFD9D9", - "light": "#FFD9D9", - "highContrast": "editor.background", - "highContrastLight": "editor.background" + "dark": "pullRequests.statusOpenForeground", + "light": "pullRequests.statusOpenForeground", + "highContrast": "editor.foreground", + "highContrastLight": "editor.foreground" }, - "description": "The foreground color of badges used for indicating that an pull requests is closed." + "description": "The foreground color used for the status badge indicating that a pull request is closed." }, { "id": "pullRequests.notification", diff --git a/webviews/editorWebview/index.css b/webviews/editorWebview/index.css index 5f8056433..bec39a1a2 100644 --- a/webviews/editorWebview/index.css +++ b/webviews/editorWebview/index.css @@ -488,22 +488,22 @@ body button .icon { .status-badge-merged { background-color: var(--vscode-pullRequests-merged); - color: var(--vscode-pullRequests-merged-foreground) + color: var(--vscode-pullRequests-status-merged-foreground); } .status-badge-open { background-color: var(--vscode-pullRequests-open); - color: var(--vscode-pullRequests-open-foreground) + color: var(--vscode-pullRequests-status-open-foreground); } .status-badge-closed { background-color: var(--vscode-pullRequests-closed); - color: var(--vscode-pullRequests-closed-foreground) + color: var(--vscode-pullRequests-status-closed-foreground); } .status-badge-draft { background-color: var(--vscode-pullRequests-draft); - color: var(--vscode-pullRequests-draft-foreground) + color: var(--vscode-pullRequests-status-draft-foreground); } .section { From 94aaa22d4c9dde41c024a48b5071ea51a3e9b382 Mon Sep 17 00:00:00 2001 From: Alex Ross Date: Fri, 19 Jan 2024 15:15:29 +0100 Subject: [PATCH 5/6] Revert "Fix light vs dark, fix var color references" This reverts commit 8dbd6c25bcf8cc5d7748565ca798f762c35d353b, reversing changes made to aaa4061cc55d9af45f8bf3cc1e0079c30ff1aca0. --- package.json | 75 +++--- package.nls.json | 9 +- src/@types/git.d.ts | 2 + .../vscode.proposed.tabInputTextMerge.d.ts | 25 ++ src/api/api.d.ts | 2 + src/common/githubRef.ts | 19 ++ src/common/settingKeys.ts | 1 + src/github/conflictGuide.ts | 173 +++++++++++++ src/github/folderRepositoryManager.ts | 53 +++- src/github/graphql.ts | 1 + src/github/interface.ts | 1 + src/github/pullRequestGitHelper.ts | 5 +- src/github/pullRequestOverview.ts | 19 +- src/github/queries.gql | 1 + src/github/queriesExtra.gql | 1 + src/github/queriesLimited.gql | 1 + src/github/utils.ts | 9 +- src/github/views.ts | 1 + src/issues/issueCompletionProvider.ts | 20 +- src/issues/issuesView.ts | 227 ++++++++++-------- src/issues/stateManager.ts | 59 ++--- .../builders/graphql/pullRequestBuilder.ts | 1 + src/test/mocks/mockRepository.ts | 8 + src/test/view/prsTree.test.ts | 2 + src/view/prsTreeDataProvider.ts | 20 +- src/view/prsTreeModel.ts | 15 ++ src/view/pullRequestCommentController.ts | 150 +++++------- src/view/reviewCommentController.ts | 6 +- src/view/reviewManager.ts | 7 + src/view/treeNodes/categoryNode.ts | 4 +- webviews/common/common.css | 5 + webviews/common/context.tsx | 4 + webviews/common/createContextNew.ts | 4 +- webviews/components/merge.tsx | 28 ++- webviews/createPullRequestViewNew/index.css | 1 + webviews/editorWebview/index.css | 5 +- .../editorWebview/test/builder/pullRequest.ts | 1 + 37 files changed, 662 insertions(+), 303 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 3b696fc96..af4f75785 100644 --- a/package.json +++ b/package.json @@ -22,13 +22,13 @@ "contribCommentEditorActionsMenu", "shareProvider", "quickDiffProvider", - "readonlyMessage", + "tabInputTextMerge", "treeViewMarkdownMessage" ], "version": "0.78.1", "publisher": "GitHub", "engines": { - "vscode": "^1.85.0" + "vscode": "^1.86.0" }, "categories": [ "Other" @@ -394,9 +394,15 @@ "default": false }, "githubPullRequests.pullPullRequestBranchBeforeCheckout": { - "type": "boolean", + "type": "string", "description": "%githubPullRequests.pullPullRequestBranchBeforeCheckout.description%", - "default": true + "enum": [ + "never", + "pull", + "pullAndMergeBase", + "pullAndUpdateBase" + ], + "default": "pull" }, "githubPullRequests.upstreamRemote": { "type": "string", @@ -540,6 +546,21 @@ "query": { "type": "string", "markdownDescription": "%githubIssues.queries.query%" + }, + "groupBy": { + "type": "array", + "markdownDescription": "%githubIssues.queries.groupBy%", + "items": { + "type": "string", + "enum": [ + "repository", + "milestone" + ], + "enumDescriptions": [ + "%githubIssues.queries.groupBy.milestone%", + "%githubIssues.queries.groupBy.repository%" + ] + } } } }, @@ -2326,13 +2347,15 @@ "comments/commentThread/comment/context": [ { "command": "pr.resolveReviewThread", - "group": "inline@3", "when": "commentController =~ /^github-(browse|review)/ && commentThread == canResolve" }, { "command": "pr.unresolveReviewThread", - "group": "inline@3", "when": "commentController =~ /^github-(browse|review)/ && commentThread == canUnresolve" + }, + { + "command": "pr.applySuggestion", + "when": "commentController =~ /^github-review/ && comment =~ /hasSuggestion/" } ], "comments/comment/title": [ @@ -2644,16 +2667,6 @@ }, "description": "The color used for indicating that a pull request is merged." }, - { - "id": "pullRequests.statusMergedForeground", - "defaults": { - "dark": "pullRequests.statusOpenForeground", - "light": "pullRequests.statusOpenForeground", - "highContrast": "editor.foreground", - "highContrastLight": "editor.foreground" - }, - "description": "The foreground color used for the status badge indicating that a pull request is merged." - }, { "id": "pullRequests.draft", "defaults": { @@ -2664,16 +2677,6 @@ }, "description": "The color used for indicating that a pull request is a draft." }, - { - "id": "pullRequests.statusDraftForeground", - "defaults": { - "dark": "pullRequests.statusOpenForeground", - "light": "pullRequests.statusOpenForeground", - "highContrast": "editor.foreground", - "highContrastLight": "editor.foreground" - }, - "description": "The foreground color used for the status badge indicating that a pull request is draft." - }, { "id": "pullRequests.open", "defaults": { @@ -2684,16 +2687,6 @@ }, "description": "The color used for indicating that a pull request is open." }, - { - "id": "pullRequests.statusOpenForeground", - "defaults": { - "dark": "#ffffff", - "light": "#000000", - "highContrast": "editor.foreground", - "highContrastLight": "editor.foreground" - }, - "description": "The foreground color used for the status badge indicating that a pull request is open." - }, { "id": "pullRequests.closed", "defaults": { @@ -2704,16 +2697,6 @@ }, "description": "The color used for indicating that a pull request is closed." }, - { - "id": "pullRequests.statusClosedForeground", - "defaults": { - "dark": "pullRequests.statusOpenForeground", - "light": "pullRequests.statusOpenForeground", - "highContrast": "editor.foreground", - "highContrastLight": "editor.foreground" - }, - "description": "The foreground color used for the status badge indicating that a pull request is closed." - }, { "id": "pullRequests.notification", "defaults": { diff --git a/package.nls.json b/package.nls.json index 46e51b4fc..b97938216 100644 --- a/package.nls.json +++ b/package.nls.json @@ -74,7 +74,11 @@ "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", + "githubPullRequests.pullPullRequestBranchBeforeCheckout.description": "Controls whether the pull request branch is pulled before checkout. Can also be set to additionally merge updates from the base branch.", + "githubPullRequests.pullPullRequestBranchBeforeCheckout.never": "Never pull the pull request branch before checkout.", + "githubPullRequests.pullPullRequestBranchBeforeCheckout.pull": "Pull the pull request branch before checkout.", + "githubPullRequests.pullPullRequestBranchBeforeCheckout.pullAndMergeBase": "Pull the pull request branch before checkout, fetch the base branch, and merge the base branch into the pull request branch.", + "githubPullRequests.pullPullRequestBranchBeforeCheckout.pullAndUpdateBase": "Pull the pull request branch before checkout, fetch the base branch, merge the base branch into the pull request branch, and finally push the pull request branch to the remote.", "githubPullRequests.upstreamRemote.description": "Controls whether an `upstream` remote is automatically added for forks", "githubPullRequests.upstreamRemote.add": "An `upstream` remote will be automatically added for forks", "githubPullRequests.upstreamRemote.never": "An `upstream` remote will never be automatically added for forks", @@ -123,6 +127,9 @@ "githubIssues.queries.markdownDescription": "Specifies what queries should be used in the GitHub issues tree using [GitHub search syntax](https://help.github.com/en/articles/understanding-the-search-syntax) with variables. The first query listed will be expanded in the Issues view. The \"default\" query includes issues assigned to you by Milestone. If you want to preserve these, make sure they are still in the array when you modify the setting.", "githubIssues.queries.label": "The label to display for the query in the Issues tree.", "githubIssues.queries.query": "The search query using [GitHub search syntax](https://help.github.com/en/articles/understanding-the-search-syntax) with variables. The variable `${user}` can be used to specify the logged in user within a search. `${owner}` and `${repository}` can be used to specify the repository by using `repo:${owner}/${repository}`.", + "githubIssues.queries.groupBy": "The categories to group issues by when displaying them, in the order in which they should be grouped", + "githubIssues.queries.groupBy.repository": "Group issues by their repository.", + "githubIssues.queries.groupBy.milestone": "Group issues by their milestone.", "githubIssues.queries.default.myIssues": "My Issues", "githubIssues.queries.default.createdIssues": "Created Issues", "githubIssues.queries.default.recentIssues": "Recent Issues", diff --git a/src/@types/git.d.ts b/src/@types/git.d.ts index b3d6fbcea..e225cc298 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 000000000..9f90ea88d --- /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 98c8fb8d2..4b4947cba 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/common/githubRef.ts b/src/common/githubRef.ts index eb6847443..867a6f083 100644 --- a/src/common/githubRef.ts +++ b/src/common/githubRef.ts @@ -3,7 +3,9 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { Remote, Repository } from '../api/api'; import { Protocol } from './protocol'; +import { parseRemote } from './remote'; export class GitHubRef { public repositoryCloneUrl: Protocol; @@ -12,3 +14,20 @@ export class GitHubRef { this.repositoryCloneUrl = new Protocol(repositoryCloneUrl); } } + +export function findLocalRepoRemoteFromGitHubRef(repository: Repository, gitHubRef: GitHubRef): Remote | undefined { + const targetRepo = gitHubRef.repositoryCloneUrl.repositoryName.toLowerCase(); + const targetOwner = gitHubRef.owner.toLowerCase(); + for (const remote of repository.state.remotes) { + const url = remote.fetchUrl ?? remote.pushUrl; + if (!url) { + continue; + } + const parsedRemote = parseRemote(remote.name, url); + const parsedOwner = parsedRemote?.owner.toLowerCase(); + const parsedRepo = parsedRemote?.repositoryName.toLowerCase(); + if (parsedOwner === targetOwner && parsedRepo === targetRepo) { + return remote; + } + } +} diff --git a/src/common/settingKeys.ts b/src/common/settingKeys.ts index 8ab9f20fd..17d63e9e2 100644 --- a/src/common/settingKeys.ts +++ b/src/common/settingKeys.ts @@ -29,6 +29,7 @@ export const SELECT_LOCAL_BRANCH = 'selectLocalBranch'; export const SELECT_REMOTE = 'selectRemote'; export const REMOTES = 'remotes'; export const PULL_PR_BRANCH_BEFORE_CHECKOUT = 'pullPullRequestBranchBeforeCheckout'; +export type PullPRBranchVariants = 'never' | 'pull' | 'pullAndMergeBase' | 'pullAndUpdateBase' | true | false; export const UPSTREAM_REMOTE = 'upstreamRemote'; export const DEFAULT_CREATE_OPTION = 'defaultCreateOption'; export const CREATE_BASE_BRANCH = 'createDefaultBaseBranch'; diff --git a/src/github/conflictGuide.ts b/src/github/conflictGuide.ts new file mode 100644 index 000000000..45008a947 --- /dev/null +++ b/src/github/conflictGuide.ts @@ -0,0 +1,173 @@ +/*--------------------------------------------------------------------------------------------- + * 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 ConflictModel implements vscode.Disposable { + public readonly startingConflictsCount: number; + private _lastReportedRemainingCount: number; + private _disposables: vscode.Disposable[] = []; + private _onConflictCountChanged: vscode.EventEmitter = new vscode.EventEmitter(); + public readonly onConflictCountChanged: vscode.Event = this._onConflictCountChanged.event; // reports difference in number of conflicts + private _finishedCommit: vscode.EventEmitter = new vscode.EventEmitter(); + public readonly message: string; + + constructor(private readonly _repository: Repository, private readonly _upstream: string, private readonly _into: string, public readonly push: boolean) { + this.startingConflictsCount = this.remainingConflicts.length; + this._lastReportedRemainingCount = 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._lastReportedRemainingCount !== remainingCount) { + this._onConflictCountChanged.fire(this._lastReportedRemainingCount - remainingCount); + this._lastReportedRemainingCount = remainingCount; + } + if (remainingCount === 0) { + this.listenForCommit(); + } + } + + private async listenForCommit() { + let localDisposable: vscode.Disposable | undefined; + const result = await 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); + }); + + localDisposable?.dispose(); + if (result && this.push) { + this._repository.push(); + } + 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); + } + } + } + } + + public 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(); + } + }); + this._disposables.push(disposable); + await this._repository.mergeAbort(); + this._finishedCommit.fire(false); + } + + private async first(): Promise { + if (this.remainingConflicts.length === 0) { + return; + } + await commands.focusView('workbench.scm'); + this._reportProgress(); + await Promise.all(this.remainingConflicts.map(conflict => commands.executeCommand('git.openMergeEditor', conflict.uri))); + } + + public static async begin(repository: Repository, upstream: string, into: string, push: boolean): Promise { + const model = new ConflictModel(repository, upstream, into, push); + if (model.remainingConflicts.length === 0) { + return undefined; + } + const notification = new ConflictNotification(model, repository); + model._disposables.push(notification); + model.first(); + return model; + } + + public finished(): Promise { + return asPromise(this._finishedCommit.event); + } + + dispose() { + dispose(this._disposables); + } +} + +class ConflictNotification implements vscode.Disposable { + private _disposables: vscode.Disposable[] = []; + + constructor(private readonly _conflictModel: ConflictModel, private readonly _repository: Repository) { + vscode.window.withProgress({ location: vscode.ProgressLocation.Notification, cancellable: true }, async (progress, token) => { + const report = (increment: number) => { + progress.report({ message: vscode.l10n.t('Use the Source Control view to resolve conflicts, {0} of {0} remaining', this._conflictModel.remainingConflicts.length, this._conflictModel.startingConflictsCount), increment }); + }; + report(0); + return new Promise((resolve) => { + this._disposables.push(this._conflictModel.onConflictCountChanged((conflictsChangedBy) => { + const increment = conflictsChangedBy * (100 / this._conflictModel.startingConflictsCount); + report(increment); + if (this._conflictModel.remainingConflicts.length === 0) { + resolve(true); + } + })); + this._disposables.push(token.onCancellationRequested(() => { + this._conflictModel.abort(); + resolve(false); + })); + }); + }).then(async (result) => { + if (result) { + const commit = vscode.l10n.t('Commit'); + const cancel = vscode.l10n.t('Abort Merge'); + let message: string; + if (this._conflictModel.push) { + message = vscode.l10n.t('All conflicts resolved. Commit and push the resolution to continue.'); + } else { + message = vscode.l10n.t('All conflicts resolved. Commit the resolution to continue.'); + } + const result = await vscode.window.showInformationMessage(message, commit, cancel); + if (result === commit) { + await this._repository.commit(this._conflictModel.message); + return true; + } else { + await this._conflictModel.abort(); + return false; + } + } + }); + } + + dispose() { + dispose(this._disposables); + } +} \ No newline at end of file diff --git a/src/github/folderRepositoryManager.ts b/src/github/folderRepositoryManager.ts index 04ff68402..f27a47143 100644 --- a/src/github/folderRepositoryManager.ts +++ b/src/github/folderRepositoryManager.ts @@ -10,6 +10,7 @@ import { GitApiImpl, GitErrorCodes } from '../api/api1'; import { GitHubManager } from '../authentication/githubServer'; import { AuthProvider, GitHubServerType } from '../common/authentication'; import { commands, contexts } from '../common/executeCommands'; +import { findLocalRepoRemoteFromGitHubRef } from '../common/githubRef'; import Logger from '../common/logger'; import { Protocol, ProtocolType } from '../common/protocol'; import { GitHubRemote, parseRemote, parseRepositoryRemotes, Remote } from '../common/remote'; @@ -32,10 +33,11 @@ import { PULL_REQUEST_OVERVIEW_VIEW_TYPE } from '../common/webview'; import { NEVER_SHOW_PULL_NOTIFICATION, REPO_KEYS, ReposState } from '../extensionState'; import { git } from '../gitProviders/gitCommands'; import { OctokitCommon } from './common'; +import { ConflictModel } from './conflictGuide'; import { CredentialStore } from './credentials'; import { GitHubRepository, ItemsData, PullRequestData, TeamReviewerRefreshKind, ViewerPermission } from './githubRepository'; import { PullRequestState, UserResponse } from './graphql'; -import { IAccount, ILabel, IMilestone, IProject, IPullRequestsPagingOptions, Issue, ITeam, MergeMethod, PRType, RepoAccessAndMergeMethods, User } from './interface'; +import { IAccount, ILabel, IMilestone, IProject, IPullRequestsPagingOptions, Issue, ITeam, MergeMethod, PRType, PullRequestMergeability, RepoAccessAndMergeMethods, User } from './interface'; import { IssueModel } from './issueModel'; import { MilestoneModel } from './milestoneModel'; import { PullRequestGitHelper, PullRequestMetadata } from './pullRequestGitHelper'; @@ -2132,6 +2134,55 @@ export class FolderRepositoryManager implements vscode.Disposable { return this.repository.checkout(branchName); } + async tryMergeBaseIntoHead(pullRequest: PullRequestModel, push: boolean): Promise { + if (await this.isHeadUpToDateWithBase(pullRequest)) { + return; + } + + if (this.repository.state.workingTreeChanges.length > 0 || this.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; + } + const baseRemote = findLocalRepoRemoteFromGitHubRef(this.repository, pullRequest.base)?.name; + if (!baseRemote) { + return; + } + const qualifiedUpstream = `${baseRemote}/${pullRequest.base.ref}`; + await vscode.window.withProgress({ location: vscode.ProgressLocation.Notification }, async (progress) => { + progress.report({ message: vscode.l10n.t('Fetching branch {0}', qualifiedUpstream) }); + await this.repository.fetch({ ref: pullRequest.base.ref, remote: baseRemote }); + progress.report({ message: vscode.l10n.t('Merging branch {0} into {1}', qualifiedUpstream, this.repository.state.HEAD!.name!) }); + try { + await this.repository.merge(qualifiedUpstream); + } catch (e) { + if (e.gitErrorCode !== GitErrorCodes.Conflict) { + throw e; + } + } + }); + + if (pullRequest.item.mergeable === PullRequestMergeability.Conflict) { + const wizard = await ConflictModel.begin(this.repository, pullRequest.base.ref, this.repository.state.HEAD!.name!, push); + await wizard?.finished(); + wizard?.dispose(); + } else { + await this.repository.push(); + } + } + + async isHeadUpToDateWithBase(pullRequestModel: PullRequestModel): Promise { + if (!pullRequestModel.head) { + return false; + } + const baseRemote = findLocalRepoRemoteFromGitHubRef(this.repository, pullRequestModel.base)?.name; + const headRemote = findLocalRepoRemoteFromGitHubRef(this.repository, pullRequestModel.head)?.name; + if (!baseRemote || !headRemote) { + return false; + } + const log = await this.repository.log({ range: `${headRemote}/${pullRequestModel.head.ref}..${baseRemote}/${pullRequestModel.base.ref}` }); + return log.length === 0; + } + async fetchById(githubRepo: GitHubRepository, id: number): Promise { const pullRequest = await githubRepo.getPullRequest(id); if (pullRequest) { diff --git a/src/github/graphql.ts b/src/github/graphql.ts index 1bf331f74..728a36277 100644 --- a/src/github/graphql.ts +++ b/src/github/graphql.ts @@ -603,6 +603,7 @@ export interface PullRequest { }; viewerCanEnableAutoMerge: boolean; viewerCanDisableAutoMerge: boolean; + viewerCanUpdate: boolean; isDraft?: boolean; suggestedReviewers: SuggestedReviewerResponse[]; projectItems?: { diff --git a/src/github/interface.ts b/src/github/interface.ts index f4acc433b..c879644c2 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; + viewerCanUpdate: boolean; autoMerge?: boolean; autoMergeMethod?: MergeMethod; allowAutoMerge?: boolean; diff --git a/src/github/pullRequestGitHelper.ts b/src/github/pullRequestGitHelper.ts index b9e3a2854..6c5cd9ec4 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 { PR_SETTINGS_NAMESPACE, PULL_PR_BRANCH_BEFORE_CHECKOUT } from '../common/settingKeys'; +import { PR_SETTINGS_NAMESPACE, PULL_PR_BRANCH_BEFORE_CHECKOUT, PullPRBranchVariants } from '../common/settingKeys'; import { IResolvedPullRequestModel, PullRequestModel } from './pullRequestModel'; const PullRequestRemoteMetadataKey = 'github-pr-remote'; @@ -198,7 +198,8 @@ export class PullRequestGitHelper { await repository.checkout(branchName); // respect the git setting to fetch before checkout - if (vscode.workspace.getConfiguration(PR_SETTINGS_NAMESPACE).get(PULL_PR_BRANCH_BEFORE_CHECKOUT, true)) { + const settingValue = vscode.workspace.getConfiguration(PR_SETTINGS_NAMESPACE).get(PULL_PR_BRANCH_BEFORE_CHECKOUT, 'pull'); + if (settingValue === 'pull' || settingValue === 'pullAndMergeBase' || settingValue === 'pullAndUpdateBase' || settingValue === true) { const remote = readConfig(`branch.${branchName}.remote`); const ref = readConfig(`branch.${branchName}.merge`); progress.report({ message: vscode.l10n.t('Fetching branch {0}', branchName) }); diff --git a/src/github/pullRequestOverview.ts b/src/github/pullRequestOverview.ts index 2e5b2ca2c..b110e5c03 100644 --- a/src/github/pullRequestOverview.ts +++ b/src/github/pullRequestOverview.ts @@ -190,7 +190,8 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel { const [ pullRequest, @@ -203,7 +204,8 @@ 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, {}); + } + await this._folderRepositoryManager.tryMergeBaseIntoHead(this._item, true); + + 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 812fc908a..8fc80dc70 100644 --- a/src/github/queries.gql +++ b/src/github/queries.gql @@ -82,6 +82,7 @@ fragment PullRequestFragment on PullRequest { } viewerCanEnableAutoMerge viewerCanDisableAutoMerge + viewerCanUpdate id databaseId isDraft diff --git a/src/github/queriesExtra.gql b/src/github/queriesExtra.gql index 9e8df8d51..930d75217 100644 --- a/src/github/queriesExtra.gql +++ b/src/github/queriesExtra.gql @@ -82,6 +82,7 @@ fragment PullRequestFragment on PullRequest { } viewerCanEnableAutoMerge viewerCanDisableAutoMerge + viewerCanUpdate id databaseId isDraft diff --git a/src/github/queriesLimited.gql b/src/github/queriesLimited.gql index df60096c4..24e513aca 100644 --- a/src/github/queriesLimited.gql +++ b/src/github/queriesLimited.gql @@ -73,6 +73,7 @@ fragment PullRequestFragment on PullRequest { } viewerCanEnableAutoMerge viewerCanDisableAutoMerge + viewerCanUpdate id databaseId isDraft diff --git a/src/github/utils.ts b/src/github/utils.ts index f5349ad27..43f3c776e 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, + viewerCanUpdate: 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, + viewerCanUpdate: graphQLPullRequest.viewerCanUpdate, labels: graphQLPullRequest.labels.nodes, isDraft: graphQLPullRequest.isDraft, suggestedReviewers: parseSuggestedReviewers(graphQLPullRequest.suggestedReviewers), @@ -803,6 +803,7 @@ export function parseGraphQLIssue(issue: GraphQL.PullRequest, githubRepository: assignees: issue.assignees?.nodes.map(assignee => parseAuthor(assignee, githubRepository)), user: parseAuthor(issue.author, githubRepository), labels: issue.labels.nodes, + milestone: parseMilestone(issue.milestone), repositoryName: issue.repository?.name ?? githubRepository.remote.repositoryName, repositoryOwner: issue.repository?.owner.login ?? githubRepository.remote.owner, repositoryUrl: issue.repository?.url ?? githubRepository.remote.url, diff --git a/src/github/views.ts b/src/github/views.ts index 373f1fd36..0c13364f8 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/issues/issueCompletionProvider.ts b/src/issues/issueCompletionProvider.ts index c1685c351..a48772a81 100644 --- a/src/issues/issueCompletionProvider.ts +++ b/src/issues/issueCompletionProvider.ts @@ -16,7 +16,7 @@ import { MilestoneModel } from '../github/milestoneModel'; import { RepositoriesManager } from '../github/repositoriesManager'; import { getIssueNumberLabel, variableSubstitution } from '../github/utils'; import { extractIssueOriginFromQuery, NEW_ISSUE_SCHEME } from './issueFile'; -import { StateManager } from './stateManager'; +import { IssueQueryResult, StateManager } from './stateManager'; import { getRootUriFromScmInputUri, isComment, @@ -152,24 +152,18 @@ export class IssueCompletionProvider implements vscode.CompletionItemProvider { // Count up total number of issues. The number of queries is expected to be small. let totalIssues = 0; for (const issueQuery of issueData) { - const issuesOrMilestones: IssueModel[] | MilestoneModel[] = (await issueQuery[1]) ?? []; - if (issuesOrMilestones[0] instanceof IssueModel) { - totalIssues += issuesOrMilestones.length; - } else { - for (const milestone of issuesOrMilestones) { - totalIssues += (milestone as MilestoneModel).issues.length; - } - } + const issuesOrMilestones: IssueQueryResult = await issueQuery[1]; + totalIssues += issuesOrMilestones.issues.length; } for (const issueQuery of issueData) { - const issuesOrMilestones: IssueModel[] | MilestoneModel[] = (await issueQuery[1]) ?? []; - if (issuesOrMilestones.length === 0) { + const issuesOrMilestones: IssueQueryResult = await issueQuery[1]; + if (issuesOrMilestones.issues.length === 0) { continue; } if (issuesOrMilestones[0] instanceof IssueModel) { let index = 0; - for (const issue of issuesOrMilestones) { + for (const issue of issuesOrMilestones.issues) { if (filterOwnerAndRepo && ((issue as IssueModel).remote.owner !== filterOwnerAndRepo.owner || (issue as IssueModel).remote.repositoryName !== filterOwnerAndRepo.repo)) { continue; } @@ -179,7 +173,7 @@ export class IssueCompletionProvider implements vscode.CompletionItemProvider { ); } } else { - for (let index = 0; index < issuesOrMilestones.length; index++) { + for (let index = 0; index < issuesOrMilestones.issues.length; index++) { const value: MilestoneModel = issuesOrMilestones[index] as MilestoneModel; for (const issue of value.issues) { if (filterOwnerAndRepo && ((issue as IssueModel).remote.owner !== filterOwnerAndRepo.owner || (issue as IssueModel).remote.repositoryName !== filterOwnerAndRepo.repo)) { diff --git a/src/issues/issuesView.ts b/src/issues/issuesView.ts index 52b1792af..bbba16ff0 100644 --- a/src/issues/issuesView.ts +++ b/src/issues/issuesView.ts @@ -6,16 +6,16 @@ import * as path from 'path'; import * as vscode from 'vscode'; import { commands, contexts } from '../common/executeCommands'; +import { groupBy } from '../common/utils'; import { FolderRepositoryManager, ReposManagerState } from '../github/folderRepositoryManager'; -import { IssueModel } from '../github/issueModel'; import { RepositoriesManager } from '../github/repositoriesManager'; import { issueBodyHasLink } from './issueLinkLookup'; -import { IssueItem, MilestoneItem, StateManager } from './stateManager'; +import { IssueItem, QueryGroup, StateManager } from './stateManager'; import { issueMarkdown } from './util'; export class IssueUriTreeItem extends vscode.TreeItem { constructor( - public readonly uri: vscode.Uri | undefined, + public readonly repoRootUri: vscode.Uri | undefined, label: string, collapsibleState?: vscode.TreeItemCollapsibleState, ) { @@ -27,13 +27,27 @@ export class IssueUriTreeItem extends vscode.TreeItem { } } +class QueryNode { + constructor( + public readonly repoRootUri: vscode.Uri, + public readonly queryLabel: string, + public readonly isFirst: boolean + ) { + } +} + +class IssueGroupNode { + constructor(public readonly repoRootUri: vscode.Uri, public readonly queryLabel, public readonly isInFirstQuery: boolean, public readonly groupLevel: number, public readonly group: string, public readonly groupByOrder: QueryGroup[], public readonly issuesInGroup: IssueItem[]) { + } +} + export class IssuesTreeData - implements vscode.TreeDataProvider { + implements vscode.TreeDataProvider { private _onDidChangeTreeData: vscode.EventEmitter< - FolderRepositoryManager | IssueItem | MilestoneItem | null | undefined | void + FolderRepositoryManager | IssueItem | null | undefined | void > = new vscode.EventEmitter(); public onDidChangeTreeData: vscode.Event< - FolderRepositoryManager | IssueItem | MilestoneItem | null | undefined | void + FolderRepositoryManager | IssueItem | null | undefined | void > = this._onDidChangeTreeData.event; constructor( @@ -59,55 +73,57 @@ export class IssuesTreeData ); } - getTreeItem(element: FolderRepositoryManager | IssueItem | MilestoneItem | IssueUriTreeItem): IssueUriTreeItem { - let treeItem: IssueUriTreeItem; - if (element instanceof IssueUriTreeItem) { - treeItem = element; - treeItem.collapsibleState = getQueryExpandState(this.context, element, element.collapsibleState); - } else if (element instanceof FolderRepositoryManager) { - treeItem = new IssueUriTreeItem( - element.repository.rootUri, - path.basename(element.repository.rootUri.fsPath), - getQueryExpandState(this.context, element, vscode.TreeItemCollapsibleState.Expanded) - ); - } else if (!(element instanceof IssueModel)) { - treeItem = new IssueUriTreeItem( - element.uri, - element.milestone.title, - getQueryExpandState(this.context, element, element.issues.length > 0 - ? vscode.TreeItemCollapsibleState.Expanded - : vscode.TreeItemCollapsibleState.None) - ); + private getFolderRepoItem(element: FolderRepositoryManager): vscode.TreeItem { + return new vscode.TreeItem(path.basename(element.repository.rootUri.fsPath), getQueryExpandState(this.context, element, vscode.TreeItemCollapsibleState.Expanded)); + } + + private getQueryItem(element: QueryNode): vscode.TreeItem { + const item = new vscode.TreeItem(element.queryLabel, getQueryExpandState(this.context, element, element.isFirst ? vscode.TreeItemCollapsibleState.Expanded : vscode.TreeItemCollapsibleState.Collapsed)); + item.contextValue = 'query'; + return item; + } + + private getIssueGroupItem(element: IssueGroupNode): vscode.TreeItem { + return new vscode.TreeItem(element.group, getQueryExpandState(this.context, element, element.isInFirstQuery ? vscode.TreeItemCollapsibleState.Expanded : vscode.TreeItemCollapsibleState.Collapsed)); + } + + private getIssueTreeItem(element: IssueItem): vscode.TreeItem { + const treeItem = new vscode.TreeItem(`${element.number}: ${element.title}`, vscode.TreeItemCollapsibleState.None); + treeItem.iconPath = element.isOpen + ? new vscode.ThemeIcon('issues', new vscode.ThemeColor('issues.open')) + : new vscode.ThemeIcon('issue-closed', new vscode.ThemeColor('issues.closed')); + if (this.stateManager.currentIssue(element.uri)?.issue.number === element.number) { + treeItem.label = `✓ ${treeItem.label as string}`; + treeItem.contextValue = 'currentissue'; } else { - treeItem = new IssueUriTreeItem( - undefined, - `${element.number}: ${element.title}`, - vscode.TreeItemCollapsibleState.None, - ); - treeItem.iconPath = element.isOpen - ? new vscode.ThemeIcon('issues', new vscode.ThemeColor('issues.open')) - : new vscode.ThemeIcon('issue-closed', new vscode.ThemeColor('issues.closed')); - if (this.stateManager.currentIssue(element.uri)?.issue.number === element.number) { - treeItem.label = `✓ ${treeItem.label as string}`; - treeItem.contextValue = 'currentissue'; + const savedState = this.stateManager.getSavedIssueState(element.number); + if (savedState.branch) { + treeItem.contextValue = 'continueissue'; } else { - const savedState = this.stateManager.getSavedIssueState(element.number); - if (savedState.branch) { - treeItem.contextValue = 'continueissue'; - } else { - treeItem.contextValue = 'issue'; - } - } - if (issueBodyHasLink(element)) { - treeItem.contextValue = 'link' + treeItem.contextValue; + treeItem.contextValue = 'issue'; } } + if (issueBodyHasLink(element)) { + treeItem.contextValue = 'link' + treeItem.contextValue; + } return treeItem; } + getTreeItem(element: FolderRepositoryManager | QueryNode | IssueGroupNode | IssueItem): vscode.TreeItem { + if (element instanceof FolderRepositoryManager) { + return this.getFolderRepoItem(element); + } else if (element instanceof QueryNode) { + return this.getQueryItem(element); + } else if (element instanceof IssueGroupNode) { + return this.getIssueGroupItem(element); + } else { + return this.getIssueTreeItem(element); + } + } + getChildren( - element: FolderRepositoryManager | IssueItem | MilestoneItem | IssueUriTreeItem | undefined, - ): FolderRepositoryManager[] | Promise<(IssueItem | MilestoneItem)[]> | IssueItem[] | IssueUriTreeItem[] { + element: FolderRepositoryManager | QueryNode | IssueGroupNode | IssueItem | undefined, + ): FolderRepositoryManager[] | QueryNode[] | Promise { if (element === undefined && this.manager.state !== ReposManagerState.RepositoriesLoaded) { return this.getStateChildren(); } else { @@ -117,15 +133,15 @@ export class IssuesTreeData async resolveTreeItem( item: vscode.TreeItem, - element: FolderRepositoryManager | IssueItem | MilestoneItem | vscode.TreeItem, + element: FolderRepositoryManager | QueryNode | IssueGroupNode | IssueItem, ): Promise { - if (element instanceof IssueModel) { + if (element instanceof IssueItem) { item.tooltip = await issueMarkdown(element, this.context, this.manager); } return item; } - getStateChildren(): IssueUriTreeItem[] { + getStateChildren(): [] { if ((this.manager.state === ReposManagerState.NeedsAuthentication) || !this.manager.folderManagers.length) { return []; @@ -135,48 +151,72 @@ export class IssuesTreeData } } - getQueryItems(folderManager: FolderRepositoryManager): Promise<(IssueItem | MilestoneItem)[]> | IssueUriTreeItem[] { - const issueCollection = this.stateManager.getIssueCollection(folderManager.repository.rootUri); - if (issueCollection.size === 1) { - return Array.from(issueCollection.values())[0]; + private getRootChildren(): FolderRepositoryManager[] | QueryNode[] | Promise { + // If there's only one folder manager go straight to the query nodes + if (this.manager.folderManagers.length === 1) { + return this.getRepoChildren(this.manager.folderManagers[0]); + } else if (this.manager.folderManagers.length > 1) { + return this.manager.folderManagers; + } else { + return []; } + } + + private getRepoChildren(folderManager: FolderRepositoryManager): QueryNode[] | Promise { + const issueCollection = this.stateManager.getIssueCollection(folderManager.repository.rootUri); const queryLabels = Array.from(issueCollection.keys()); - const firstLabel = queryLabels[0]; - return queryLabels.map(label => { - const item = new IssueUriTreeItem(folderManager.repository.rootUri, label); - item.contextValue = 'query'; - item.collapsibleState = - label === firstLabel - ? vscode.TreeItemCollapsibleState.Expanded - : vscode.TreeItemCollapsibleState.Collapsed; + if (queryLabels.length === 1) { + return this.getQueryNodeChildren(new QueryNode(folderManager.repository.rootUri, queryLabels[0], true)); + } + return queryLabels.map((label, index) => { + const item = new QueryNode(folderManager.repository.rootUri, label, index === 0); return item; }); } - getIssuesChildren( - element: FolderRepositoryManager | IssueItem | MilestoneItem | IssueUriTreeItem | undefined, - ): FolderRepositoryManager[] | Promise<(IssueItem | MilestoneItem)[]> | IssueItem[] | IssueUriTreeItem[] { - if (element === undefined) { - // If there's only one query, don't display a title for it - if (this.manager.folderManagers.length === 1) { - return this.getQueryItems(this.manager.folderManagers[0]); - } else if (this.manager.folderManagers.length > 1) { - return this.manager.folderManagers; + private async getQueryNodeChildren(queryNode: QueryNode): Promise { + const issueCollection = this.stateManager.getIssueCollection(queryNode.repoRootUri); + const issueQueryResult = await issueCollection.get(queryNode.queryLabel); + if (!issueQueryResult) { + return []; + } + return this.getIssueGroupsForGroupIndex(queryNode.repoRootUri, queryNode.queryLabel, queryNode.isFirst, issueQueryResult.groupBy, 0, issueQueryResult.issues); + } + + private getIssueGroupsForGroupIndex(repoRootUri: vscode.Uri, queryLabel: string, isFirst: boolean, groupByOrder: QueryGroup[], index: number, issues: IssueItem[]): IssueGroupNode[] | IssueItem[] { + if (groupByOrder.length <= index) { + return issues; + } + const groupByValue = groupByOrder[index]; + const groups = groupBy(issues, issue => { + if (groupByValue === 'repository') { + return `${issue.remote.owner}/${issue.remote.repositoryName}`; } else { - return []; + return issue.milestone?.title ?? 'No Milestone'; } + }); + const nodes: IssueGroupNode[] = []; + for (const group in groups) { + nodes.push(new IssueGroupNode(repoRootUri, queryLabel, isFirst, index, group, groupByOrder, groups[group])); + } + return nodes; + } + + private async getIssueGroupChildren(issueGroupNode: IssueGroupNode): Promise { + return this.getIssueGroupsForGroupIndex(issueGroupNode.repoRootUri, issueGroupNode.queryLabel, issueGroupNode.isInFirstQuery, issueGroupNode.groupByOrder, issueGroupNode.groupLevel + 1, issueGroupNode.issuesInGroup); + } + + getIssuesChildren( + element: FolderRepositoryManager | QueryNode | IssueGroupNode | IssueItem | undefined, + ): FolderRepositoryManager[] | QueryNode[] | Promise { + if (element === undefined) { + return this.getRootChildren(); } else if (element instanceof FolderRepositoryManager) { - return this.getQueryItems(element); - } else if (element instanceof IssueUriTreeItem) { - return element.uri - ? this.stateManager.getIssueCollection(element.uri).get(element.labelAsString!) ?? [] - : []; - } else if (!(element instanceof IssueModel)) { - return element.issues.map(item => { - const issueItem: IssueItem = Object.assign(item); - issueItem.uri = element.uri; - return issueItem; - }); + return this.getRepoChildren(element); + } else if (element instanceof QueryNode) { + return this.getQueryNodeChildren(element); + } else if (element instanceof IssueGroupNode) { + return this.getIssueGroupChildren(element); } else { return []; } @@ -185,24 +225,19 @@ export class IssuesTreeData const EXPANDED_ISSUES_STATE = 'expandedIssuesState'; -function expandStateId(element: FolderRepositoryManager | IssueItem | MilestoneItem | IssueUriTreeItem | vscode.TreeItem | undefined) { - if (!element) { - return; - } +function expandStateId(element: FolderRepositoryManager | QueryNode | IssueGroupNode | IssueItem) { let id: string | undefined; - if (element instanceof IssueUriTreeItem) { - id = element.labelAsString; - } else if (element instanceof vscode.TreeItem) { - // No id needed - } else if (element instanceof FolderRepositoryManager) { + if (element instanceof FolderRepositoryManager) { id = element.repository.rootUri.toString(); - } else if (!(element instanceof IssueModel)) { - id = element.milestone.title; + } else if (element instanceof QueryNode) { + id = `${element.repoRootUri.toString()}/${element.queryLabel}`; + } else if (element instanceof IssueGroupNode) { + id = `${element.repoRootUri.toString()}/${element.queryLabel}/${element.groupLevel}/${element.group}`; } return id; } -export function updateExpandedQueries(context: vscode.ExtensionContext, element: FolderRepositoryManager | IssueItem | MilestoneItem | IssueUriTreeItem | vscode.TreeItem | undefined, isExpanded: boolean) { +export function updateExpandedQueries(context: vscode.ExtensionContext, element: FolderRepositoryManager | QueryNode | IssueGroupNode | IssueItem, isExpanded: boolean) { const id = expandStateId(element); if (id) { @@ -216,7 +251,7 @@ export function updateExpandedQueries(context: vscode.ExtensionContext, element: } } -function getQueryExpandState(context: vscode.ExtensionContext, element: FolderRepositoryManager | IssueItem | MilestoneItem | IssueUriTreeItem | undefined, defaultState: vscode.TreeItemCollapsibleState = vscode.TreeItemCollapsibleState.Collapsed): vscode.TreeItemCollapsibleState { +function getQueryExpandState(context: vscode.ExtensionContext, element: FolderRepositoryManager | QueryNode | IssueGroupNode, defaultState: vscode.TreeItemCollapsibleState = vscode.TreeItemCollapsibleState.Expanded): vscode.TreeItemCollapsibleState { const id = expandStateId(element); if (id) { const savedValue = context.workspaceState.get(EXPANDED_ISSUES_STATE); diff --git a/src/issues/stateManager.ts b/src/issues/stateManager.ts index 2c0ba4335..c4eccf29c 100644 --- a/src/issues/stateManager.ts +++ b/src/issues/stateManager.ts @@ -49,7 +49,8 @@ interface IssuesState { branches: Record; } -const DEFAULT_QUERY_CONFIGURATION_VALUE = [{ label: vscode.l10n.t('My Issues'), query: 'default' }]; +// eslint-disable-next-line no-template-curly-in-string +const DEFAULT_QUERY_CONFIGURATION_VALUE: { label: string, query: string, groupBy: QueryGroup[] }[] = [{ label: vscode.l10n.t('My Issues'), query: 'is:open assignee:@me repo:${owner}/${repository}', groupBy: ['milestone'] }]; export interface MilestoneItem extends MilestoneModel { uri: vscode.Uri; @@ -63,12 +64,19 @@ interface SingleRepoState { lastHead?: string; lastBranch?: string; currentIssue?: CurrentIssue; - issueCollection: Map>; + issueCollection: Map>; maxIssueNumber: number; userMap?: Promise>; folderManager: FolderRepositoryManager; } +export type QueryGroup = 'repository' | 'milestone'; + +export interface IssueQueryResult { + groupBy: QueryGroup[]; + issues: IssueItem[]; +} + export class StateManager { public readonly resolvedIssues: Map> = new Map(); private _singleRepoStates: Map = new Map(); @@ -76,14 +84,14 @@ export class StateManager { public onRefreshCacheNeeded: vscode.Event = this._onRefreshCacheNeeded.event; private _onDidChangeIssueData: vscode.EventEmitter = new vscode.EventEmitter(); public onDidChangeIssueData: vscode.Event = this._onDidChangeIssueData.event; - private _queries: { label: string; query: string }[] = []; + private _queries: { label: string; query: string, groupBy?: QueryGroup[] }[] = []; private _onDidChangeCurrentIssue: vscode.EventEmitter = new vscode.EventEmitter(); public readonly onDidChangeCurrentIssue: vscode.Event = this._onDidChangeCurrentIssue.event; private initializePromise: Promise | undefined; private statusBarItem?: vscode.StatusBarItem; - getIssueCollection(uri: vscode.Uri): Map> { + getIssueCollection(uri: vscode.Uri): Map> { let collection = this._singleRepoStates.get(uri.path)?.issueCollection; if (collection) { return collection; @@ -291,33 +299,26 @@ export class StateManager { private async setIssueData(folderManager: FolderRepositoryManager) { const singleRepoState = this.getOrCreateSingleRepoState(folderManager.repository.rootUri, folderManager); singleRepoState.issueCollection.clear(); - let defaults: PullRequestDefaults | undefined; - let user: string | undefined; - for (const query of this._queries) { - let items: Promise; + const enterpriseRemotes = parseRepositoryRemotes(folderManager.repository).filter( + remote => remote.isEnterprise + ); + const user = await this.getCurrentUser(enterpriseRemotes.length ? AuthProvider.githubEnterprise : AuthProvider.github); + + for (let query of this._queries) { + let items: Promise | undefined; if (query.query === DEFAULT) { - items = this.setMilestones(folderManager); - } else { - if (!defaults) { - try { - defaults = await folderManager.getPullRequestDefaults(); - } catch (e) { - // leave defaults undefined - } - } - if (!user) { - const enterpriseRemotes = parseRepositoryRemotes(folderManager.repository).filter( - remote => remote.isEnterprise - ); - user = await this.getCurrentUser(enterpriseRemotes.length ? AuthProvider.githubEnterprise : AuthProvider.github); - } - items = this.setIssues( - folderManager, - // Do not resolve pull request defaults as they will get resolved in the query later per repository - await variableSubstitution(query.query, undefined, undefined, user), - ); + query = DEFAULT_QUERY_CONFIGURATION_VALUE[0]; + } + + items = this.setIssues( + folderManager, + // Do not resolve pull request defaults as they will get resolved in the query later per repository + await variableSubstitution(query.query, undefined, undefined, user), + ).then(issues => ({ groupBy: query.groupBy ?? [], issues })); + + if (items) { + singleRepoState.issueCollection.set(query.label, items); } - singleRepoState.issueCollection.set(query.label, items); } singleRepoState.maxIssueNumber = await folderManager.getMaxIssue(); singleRepoState.lastHead = folderManager.repository.state.HEAD?.commit; diff --git a/src/test/builders/graphql/pullRequestBuilder.ts b/src/test/builders/graphql/pullRequestBuilder.ts index 1483f95fc..4e5dedd96 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 }, + viewerCanUpdate: { default: false }, commits: createLink()({ nodes: { default: [ diff --git a/src/test/mocks/mockRepository.ts b/src/test/mocks/mockRepository.ts index 8d719ac72..60c051c51 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/src/test/view/prsTree.test.ts b/src/test/view/prsTree.test.ts index e22fcc5b7..16aa66ae8 100644 --- a/src/test/view/prsTree.test.ts +++ b/src/test/view/prsTree.test.ts @@ -29,6 +29,7 @@ import { LoggingOctokit, RateLogger } from '../../github/loggingOctokit'; import { GitHubServerType } from '../../common/authentication'; import { DataUri } from '../../common/uri'; import { IAccount, ITeam } from '../../github/interface'; +import { asPromise } from '../../common/utils'; describe('GitHub Pull Requests view', function () { let sinon: SinonSandbox; @@ -196,6 +197,7 @@ describe('GitHub Pull Requests view', function () { // Need to call getChildren twice to get past the quick render with an empty list await localNode!.getChildren(); + await asPromise(provider.prsTreeModel.onLoaded); const localChildren = await localNode!.getChildren(); assert.strictEqual(localChildren.length, 2); const [localItem0, localItem1] = await Promise.all(localChildren.map(node => node.getTreeItem())); diff --git a/src/view/prsTreeDataProvider.ts b/src/view/prsTreeDataProvider.ts index 83231e881..018cedbc2 100644 --- a/src/view/prsTreeDataProvider.ts +++ b/src/view/prsTreeDataProvider.ts @@ -41,7 +41,7 @@ export class PullRequestsTreeDataProvider implements vscode.TreeDataProvider; private _initialized: boolean = false; public notificationProvider: NotificationProvider; - private _prsTreeModel: PrsTreeModel; + public readonly prsTreeModel: PrsTreeModel; get view(): vscode.TreeView { return this._view; @@ -49,14 +49,14 @@ export class PullRequestsTreeDataProvider implements vscode.TreeDataProvider folderManager ? this.refreshRepo(folderManager) : this.refresh())); - this._disposables.push(new PRStatusDecorationProvider(this._prsTreeModel)); + this.prsTreeModel = new PrsTreeModel(this._telemetry, this._reposManager, _context); + this._disposables.push(this.prsTreeModel); + this._disposables.push(this.prsTreeModel.onDidChangeData(folderManager => folderManager ? this.refreshRepo(folderManager) : this.refresh())); + this._disposables.push(new PRStatusDecorationProvider(this.prsTreeModel)); this._disposables.push(vscode.window.registerFileDecorationProvider(DecorationProvider)); this._disposables.push( vscode.commands.registerCommand('pr.refreshList', _ => { - this._prsTreeModel.clearCache(); + this.prsTreeModel.clearCache(); this._onDidChangeTreeData.fire(); }), ); @@ -111,10 +111,10 @@ export class PullRequestsTreeDataProvider implements vscode.TreeDataProvider { - this._prsTreeModel.updateExpandedQueries(expanded.element, true); + this.prsTreeModel.updateExpandedQueries(expanded.element, true); })); this._disposables.push(this._view.onDidCollapseElement(collapsed => { - this._prsTreeModel.updateExpandedQueries(collapsed.element, false); + this.prsTreeModel.updateExpandedQueries(collapsed.element, false); })); } @@ -263,7 +263,7 @@ export class PullRequestsTreeDataProvider implements vscode.TreeDataProvider = new vscode.EventEmitter(); public readonly onDidChangeData = this._onDidChangeData.event; private _expandedQueries: Set = new Set(); + private _hasLoaded: boolean = false; + private _onLoaded: vscode.EventEmitter = new vscode.EventEmitter(); + public readonly onLoaded = this._onLoaded.event; // Key is identifier from createPRNodeUri private readonly _queriedPullRequests: Map = new Map(); @@ -95,6 +98,15 @@ export class PrsTreeModel implements vscode.Disposable { return this._expandedQueries; } + get hasLoaded(): boolean { + return this._hasLoaded; + } + + private set hasLoaded(value: boolean) { + this._hasLoaded = value; + this._onLoaded.fire(); + } + public cachedPRStatus(identifier: string): PRStatusChange | undefined { return this._queriedPullRequests.get(identifier); } @@ -188,6 +200,7 @@ export class PrsTreeModel implements vscode.Disposable { this._telemetry.sendTelemetryEvent('pr.expand.local'); // Don't await this._getChecks. It fires an event that will be listened to. this._getChecks(prs); + this.hasLoaded = true; return { hasMorePages: false, hasUnsearchedRepositories: false, items: prs }; } @@ -210,6 +223,7 @@ export class PrsTreeModel implements vscode.Disposable { this._telemetry.sendTelemetryEvent('pr.expand.query'); // Don't await this._getChecks. It fires an event that will be listened to. this._getChecks(prs.items); + this.hasLoaded = true; return prs; } @@ -231,6 +245,7 @@ export class PrsTreeModel implements vscode.Disposable { this._telemetry.sendTelemetryEvent('pr.expand.all'); // Don't await this._getChecks. It fires an event that will be listened to. this._getChecks(prs.items); + this.hasLoaded = true; return prs; } diff --git a/src/view/pullRequestCommentController.ts b/src/view/pullRequestCommentController.ts index 205d03afc..ad6b9f12c 100644 --- a/src/view/pullRequestCommentController.ts +++ b/src/view/pullRequestCommentController.ts @@ -9,7 +9,7 @@ import * as vscode from 'vscode'; import { CommentHandler, registerCommentHandler, unregisterCommentHandler } from '../commentHandlerResolver'; import { DiffSide, IComment, SubjectType } from '../common/comment'; import { fromPRUri, Schemes } from '../common/uri'; -import { groupBy } from '../common/utils'; +import { dispose, groupBy } from '../common/utils'; import { FolderRepositoryManager } from '../github/folderRepositoryManager'; import { GHPRComment, GHPRCommentThread, TemporaryComment } from '../github/prComment'; import { PullRequestModel, ReviewThreadChangeEvent } from '../github/pullRequestModel'; @@ -28,12 +28,6 @@ export class PullRequestCommentController implements CommentHandler, CommentReac private _pendingCommentThreadAdds: GHPRCommentThread[] = []; private _commentHandlerId: string; private _commentThreadCache: { [key: string]: GHPRCommentThread[] } = {}; - private _openPREditors: vscode.TextEditor[] = []; - /** - * Cached threads belong to editors that are closed, but that we keep cached because they were recently used. - * This prevents comment replies that haven't been submitted from getting deleted too easily. - */ - private _closedEditorCachedThreads: Set = new Set(); private _disposables: vscode.Disposable[] = []; private readonly _context: vscode.ExtensionContext; @@ -63,9 +57,9 @@ export class PullRequestCommentController implements CommentHandler, CommentReac this._disposables.push(this.pullRequestModel.onDidChangeReviewThreads(e => this.onDidChangeReviewThreads(e))); this._disposables.push( - vscode.window.onDidChangeVisibleTextEditors(async e => { - return this.onDidChangeOpenEditors(e); - }), + vscode.window.tabGroups.onDidChangeTabs(async e => { + return this.onDidChangeOpenTabs(e); + }) ); this._disposables.push( @@ -103,51 +97,48 @@ export class PullRequestCommentController implements CommentHandler, CommentReac this.setContextKey(this.pullRequestModel.hasPendingReview); } - private getPREditors(editors: readonly vscode.TextEditor[]): vscode.TextEditor[] { - return editors.filter(editor => { - if (editor.document.uri.scheme !== Schemes.Pr) { - return false; + private async getPREditors(editors: readonly vscode.TextEditor[] | readonly (vscode.TabInputText | vscode.TabInputTextDiff)[]): Promise { + const prDocuments: Promise[] = []; + const isPrEditor = (potentialEditor: { uri: vscode.Uri, editor?: vscode.TextEditor }): Thenable | undefined => { + const params = fromPRUri(potentialEditor.uri); + if (params && params.prNumber === this.pullRequestModel.number) { + if (potentialEditor.editor) { + return Promise.resolve(potentialEditor.editor.document); + } else { + return vscode.workspace.openTextDocument(potentialEditor.uri); + } } - - const params = fromPRUri(editor.document.uri); - - if (!params || params.prNumber !== this.pullRequestModel.number) { - return false; + }; + for (const editor of editors) { + const testUris: { uri: vscode.Uri, editor?: vscode.TextEditor }[] = []; + if (editor instanceof vscode.TabInputText) { + testUris.push({ uri: editor.uri }); + } else if (editor instanceof vscode.TabInputTextDiff) { + testUris.push({ uri: editor.original }, { uri: editor.modified }); + } else { + testUris.push({ uri: editor.document.uri, editor }); } - - return true; - }); + prDocuments.push(...testUris.map(isPrEditor).filter>((doc): doc is Promise => !!doc)); + } + return Promise.all(prDocuments); } private getCommentThreadCacheKey(fileName: string, isBase: boolean): string { return `${fileName}-${isBase ? 'original' : 'modified'}`; } - private tryUsedCachedEditor(editors: vscode.TextEditor[]): vscode.TextEditor[] { - const uncachedEditors: vscode.TextEditor[] = []; - editors.forEach(editor => { - const { fileName, isBase } = fromPRUri(editor.document.uri)!; - const key = this.getCommentThreadCacheKey(fileName, isBase); - if (this._closedEditorCachedThreads.has(key)) { - // Update position in cache - this._closedEditorCachedThreads.delete(key); - this._closedEditorCachedThreads.add(key); - } else { - uncachedEditors.push(editor); - } - }); - return uncachedEditors; - } - - private async addThreadsForEditors(newEditors: vscode.TextEditor[]): Promise { - const editors = this.tryUsedCachedEditor(newEditors); + private async addThreadsForEditors(documents: vscode.TextDocument[]): Promise { const reviewThreads = this.pullRequestModel.reviewThreadsCache; const threadsByPath = groupBy(reviewThreads, thread => thread.path); const currentUser = await this._folderReposManager.getCurrentUser(); - editors.forEach(editor => { - const { fileName, isBase } = fromPRUri(editor.document.uri)!; + for (const document of documents) { + const { fileName, isBase } = fromPRUri(document.uri)!; + const cacheKey = this.getCommentThreadCacheKey(fileName, isBase); + if (this._commentThreadCache[cacheKey]) { + continue; + } if (threadsByPath[fileName]) { - this._commentThreadCache[this.getCommentThreadCacheKey(fileName, isBase)] = threadsByPath[fileName] + this._commentThreadCache[cacheKey] = threadsByPath[fileName] .filter( thread => ((thread.diffSide === DiffSide.LEFT && isBase) || @@ -156,11 +147,11 @@ export class PullRequestCommentController implements CommentHandler, CommentReac ) .map(thread => { const endLine = thread.endLine - 1; - const range = thread.subjectType === SubjectType.FILE ? undefined : threadRange(thread.startLine - 1, endLine, editor.document.lineAt(endLine).range.end.character); + const range = thread.subjectType === SubjectType.FILE ? undefined : threadRange(thread.startLine - 1, endLine, document.lineAt(endLine).range.end.character); return createVSCodeCommentThreadForReviewThread( this._context, - editor.document.uri, + document.uri, range, thread, this._commentController, @@ -169,56 +160,40 @@ export class PullRequestCommentController implements CommentHandler, CommentReac ); }); } - }); + } } private async initializeThreadsInOpenEditors(): Promise { - const prEditors = this.getPREditors(vscode.window.visibleTextEditors); - this._openPREditors = prEditors; + const prEditors = await this.getPREditors(vscode.window.visibleTextEditors); return this.addThreadsForEditors(prEditors); } - private cleanCachedEditors() { - // Keep the most recent 8 editors (4 diffs) around and clean up the rest. - if (this._closedEditorCachedThreads.size > 8) { - const keys = Array.from(this._closedEditorCachedThreads.keys()); - for (let i = 0; i < this._closedEditorCachedThreads.size - 4; i++) { - const key = keys[i]; - this.cleanCachedEditor(key); - this._closedEditorCachedThreads.delete(key); - } - } + private allTabs(): (vscode.TabInputText | vscode.TabInputTextDiff)[] { + return this.filterTabsToPrTabs(vscode.window.tabGroups.all.map(group => group.tabs).flat()); } - private cleanCachedEditor(key: string) { - const threads = this._commentThreadCache[key] || []; - threads.forEach(t => t.dispose()); - delete this._commentThreadCache[key]; + private filterTabsToPrTabs(tabs: readonly vscode.Tab[]): (vscode.TabInputText | vscode.TabInputTextDiff)[] { + return tabs.filter(tab => tab.input instanceof vscode.TabInputText || tab.input instanceof vscode.TabInputTextDiff).map(tab => tab.input as vscode.TabInputText | vscode.TabInputTextDiff); } - private addCachedEditors(editors: vscode.TextEditor[]) { - editors.forEach(editor => { - const { fileName, isBase } = fromPRUri(editor.document.uri)!; - const key = this.getCommentThreadCacheKey(fileName, isBase); - if (this._closedEditorCachedThreads.has(key)) { - // Delete to update position in the cache - this._closedEditorCachedThreads.delete(key); - } - this._closedEditorCachedThreads.add(key); - }); + private async cleanClosedPrs() { + // Remove comments for which no editors belonging to the same PR are open + const allPrEditors = await this.getPREditors(this.allTabs()); + if (allPrEditors.length === 0) { + this.removeAllCommentsThreads(); + } } - private async onDidChangeOpenEditors(editors: readonly vscode.TextEditor[]): Promise { - const prEditors = this.getPREditors(editors); - const removed = this._openPREditors.filter(x => !prEditors.includes(x)); - this.addCachedEditors(removed); - this.cleanCachedEditors(); - - const added = prEditors.filter(x => !this._openPREditors.includes(x)); - this._openPREditors = prEditors; + private async onDidChangeOpenTabs(e: vscode.TabChangeEvent): Promise { + const added = await this.getPREditors(this.filterTabsToPrTabs(e.opened)); if (added.length) { await this.addThreadsForEditors(added); } + if (e.closed.length > 0) { + // Delay cleaning closed editors to handle the case where a preview tab is replaced + await new Promise(resolve => setTimeout(resolve, 100)); + await this.cleanClosedPrs(); + } } private onDidChangeReviewThreads(e: ReviewThreadChangeEvent): void { @@ -238,9 +213,9 @@ export class PullRequestCommentController implements CommentHandler, CommentReac updateThreadWithRange(this._context, newThread, thread, this.pullRequestModel.githubRepository); this._pendingCommentThreadAdds.splice(index, 1); } else { - const openPREditors = this.getPREditors(vscode.window.visibleTextEditors); + const openPREditors = await this.getPREditors(vscode.window.visibleTextEditors); const matchingEditor = openPREditors.find(editor => { - const query = fromPRUri(editor.document.uri); + const query = fromPRUri(editor.uri); const sameSide = (thread.diffSide === DiffSide.RIGHT && !query?.isBase) || (thread.diffSide === DiffSide.LEFT && query?.isBase); @@ -249,11 +224,11 @@ export class PullRequestCommentController implements CommentHandler, CommentReac if (matchingEditor) { const endLine = thread.endLine - 1; - const range = thread.subjectType === SubjectType.FILE ? undefined : threadRange(thread.startLine - 1, endLine, matchingEditor.document.lineAt(endLine).range.end.character); + const range = thread.subjectType === SubjectType.FILE ? undefined : threadRange(thread.startLine - 1, endLine, matchingEditor.lineAt(endLine).range.end.character); newThread = createVSCodeCommentThreadForReviewThread( this._context, - matchingEditor.document.uri, + matchingEditor.uri, range, thread, this._commentController, @@ -542,11 +517,14 @@ export class PullRequestCommentController implements CommentHandler, CommentReac vscode.commands.executeCommand('setContext', 'prInDraft', inDraftMode); } - dispose() { + private removeAllCommentsThreads(): void { Object.keys(this._commentThreadCache).forEach(key => { - this._commentThreadCache[key].forEach(thread => thread.dispose()); + dispose(this._commentThreadCache[key]); }); + } + dispose() { + this.removeAllCommentsThreads(); unregisterCommentHandler(this._commentHandlerId); this._disposables.forEach(d => d.dispose()); diff --git a/src/view/reviewCommentController.ts b/src/view/reviewCommentController.ts index 235991eb2..31ce0ee9e 100644 --- a/src/view/reviewCommentController.ts +++ b/src/view/reviewCommentController.ts @@ -13,7 +13,7 @@ import { getCommentingRanges } from '../common/commentingRanges'; import { mapNewPositionToOld, mapOldPositionToNew } from '../common/diffPositionMapping'; import { GitChangeType } from '../common/file'; import Logger from '../common/logger'; -import { PR_SETTINGS_NAMESPACE, PULL_BRANCH, PULL_PR_BRANCH_BEFORE_CHECKOUT } from '../common/settingKeys'; +import { PR_SETTINGS_NAMESPACE, PULL_BRANCH, PULL_PR_BRANCH_BEFORE_CHECKOUT, PullPRBranchVariants } from '../common/settingKeys'; import { fromReviewUri, ReviewUriParams, Schemes, toReviewUri } from '../common/uri'; import { dispose, formatError, groupBy, uniqBy } from '../common/utils'; import { FolderRepositoryManager } from '../github/folderRepositoryManager'; @@ -511,7 +511,9 @@ export class ReviewCommentController Logger.error(`Failed to get content diff. ${formatError(e)}`); if ((e.stderr as string | undefined)?.includes('bad object')) { if (this._repository.state.HEAD?.upstream && retry) { - const pullSetting = vscode.workspace.getConfiguration(PR_SETTINGS_NAMESPACE).get(PULL_PR_BRANCH_BEFORE_CHECKOUT, true) && (vscode.workspace.getConfiguration(PR_SETTINGS_NAMESPACE).get<'never' | 'prompt' | 'always'>(PULL_BRANCH, 'prompt') === 'always'); + const pullBeforeCheckoutSetting = vscode.workspace.getConfiguration(PR_SETTINGS_NAMESPACE).get(PULL_PR_BRANCH_BEFORE_CHECKOUT, 'pull'); + const pullSetting = (pullBeforeCheckoutSetting === 'pull' || pullBeforeCheckoutSetting === 'pullAndMergeBase' || pullBeforeCheckoutSetting === 'pullAndUpdateBase' || pullBeforeCheckoutSetting === true) + && (vscode.workspace.getConfiguration(PR_SETTINGS_NAMESPACE).get<'never' | 'prompt' | 'always'>(PULL_BRANCH, 'prompt') === 'always'); if (pullSetting) { try { await this._repository.pull(); diff --git a/src/view/reviewManager.ts b/src/view/reviewManager.ts index 8b0645d9f..145c7fac7 100644 --- a/src/view/reviewManager.ts +++ b/src/view/reviewManager.ts @@ -21,6 +21,8 @@ import { OPEN_VIEW, POST_CREATE, PR_SETTINGS_NAMESPACE, + PULL_PR_BRANCH_BEFORE_CHECKOUT, + PullPRBranchVariants, QUICK_DIFF, } from '../common/settingKeys'; import { getReviewMode } from '../common/settingsUtils'; @@ -922,6 +924,11 @@ export class ReviewManager { await this._folderRepoManager.fetchAndCheckout(pr, progress); } }); + const updateBaseSetting = vscode.workspace.getConfiguration(PR_SETTINGS_NAMESPACE).get(PULL_PR_BRANCH_BEFORE_CHECKOUT, 'pull'); + if (updateBaseSetting === 'pullAndMergeBase' || updateBaseSetting === 'pullAndUpdateBase') { + await this._folderRepoManager.tryMergeBaseIntoHead(pr, updateBaseSetting === 'pullAndUpdateBase'); + } + } catch (e) { Logger.error(`Checkout failed #${JSON.stringify(e)}`, this.id); this.switchingToReviewMode = false; diff --git a/src/view/treeNodes/categoryNode.ts b/src/view/treeNodes/categoryNode.ts index 88ad682c5..ab030f248 100644 --- a/src/view/treeNodes/categoryNode.ts +++ b/src/view/treeNodes/categoryNode.ts @@ -124,7 +124,6 @@ export class CategoryTreeNode extends TreeNode implements vscode.TreeItem { public repositoryPageInformation: Map = new Map(); public contextValue: string; public readonly id: string = ''; - private _firstLoad: boolean = true; constructor( public parent: TreeNodeParent, @@ -306,8 +305,7 @@ export class CategoryTreeNode extends TreeNode implements vscode.TreeItem { async getChildren(): Promise { await super.getChildren(); - if (this._firstLoad) { - this._firstLoad = false; + if (!this._prsTreeModel.hasLoaded) { this.doGetChildren().then(() => this.refresh(this)); return []; } diff --git a/webviews/common/common.css b/webviews/common/common.css index 5f10120f5..f97be3055 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 71d01b1f3..7a821f1df 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/common/createContextNew.ts b/webviews/common/createContextNew.ts index b505c569b..7355aab29 100644 --- a/webviews/common/createContextNew.ts +++ b/webviews/common/createContextNew.ts @@ -267,7 +267,9 @@ export class CreatePRContextNew { if (this.createParams.autoMerge === undefined) { message.params.autoMerge = message.params.autoMergeDefault; message.params.autoMergeMethod = message.params.defaultMergeMethod; - message.params.isDraft = false; + if (message.params.autoMerge) { + message.params.isDraft = false; + } } else { message.params.autoMerge = this.createParams.autoMerge; message.params.autoMergeMethod = this.createParams.autoMergeMethod; diff --git a/webviews/components/merge.tsx b/webviews/components/merge.tsx index 0b4d1eee9..be37bfa64 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/createPullRequestViewNew/index.css b/webviews/createPullRequestViewNew/index.css index 329f579a5..15d7f0fef 100644 --- a/webviews/createPullRequestViewNew/index.css +++ b/webviews/createPullRequestViewNew/index.css @@ -183,6 +183,7 @@ textarea { padding: 5px; border: 1px solid var(--vscode-inputValidation-errorBorder); background-color: var(--vscode-inputValidation-errorBackground); + color: var(--vscode-inputValidation-errorForeground); } .below-input-error { diff --git a/webviews/editorWebview/index.css b/webviews/editorWebview/index.css index 0e44debcc..44037dc6c 100644 --- a/webviews/editorWebview/index.css +++ b/webviews/editorWebview/index.css @@ -463,6 +463,7 @@ body button .icon { #status { box-sizing: border-box; line-height: 18px; + color: var(--vscode-button-foreground); border-radius: 18px; padding: 4px 12px; margin-right: 10px; @@ -487,22 +488,18 @@ body button .icon { .status-badge-merged { background-color: var(--vscode-pullRequests-merged); - color: var(--vscode-pullRequests-statusMergedForeground); } .status-badge-open { background-color: var(--vscode-pullRequests-open); - color: var(--vscode-pullRequests-statusOpenForeground); } .status-badge-closed { background-color: var(--vscode-pullRequests-closed); - color: var(--vscode-pullRequests-statusClosedForeground); } .status-badge-draft { background-color: var(--vscode-pullRequests-draft); - color: var(--vscode-pullRequests-statusDraftForeground); } .section { diff --git a/webviews/editorWebview/test/builder/pullRequest.ts b/webviews/editorWebview/test/builder/pullRequest.ts index aae821241..19ac7fa75 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 }, From decdb2d57dae6ea8307a3b710ce87c20fa9c4e91 Mon Sep 17 00:00:00 2001 From: Alex Ross Date: Fri, 19 Jan 2024 15:17:56 +0100 Subject: [PATCH 6/6] Fix merge commit mistake --- package.json | 40 ++++++++++++++++++++++++++++++++ webviews/editorWebview/index.css | 5 +++- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index af4f75785..c6bd57a5c 100644 --- a/package.json +++ b/package.json @@ -2667,6 +2667,16 @@ }, "description": "The color used for indicating that a pull request is merged." }, + { + "id": "pullRequests.statusMergedForeground", + "defaults": { + "dark": "pullRequests.statusOpenForeground", + "light": "pullRequests.statusOpenForeground", + "highContrast": "editor.foreground", + "highContrastLight": "editor.foreground" + }, + "description": "The foreground color used for the status badge indicating that a pull request is merged." + }, { "id": "pullRequests.draft", "defaults": { @@ -2677,6 +2687,16 @@ }, "description": "The color used for indicating that a pull request is a draft." }, + { + "id": "pullRequests.statusDraftForeground", + "defaults": { + "dark": "pullRequests.statusOpenForeground", + "light": "pullRequests.statusOpenForeground", + "highContrast": "editor.foreground", + "highContrastLight": "editor.foreground" + }, + "description": "The foreground color used for the status badge indicating that a pull request is draft." + }, { "id": "pullRequests.open", "defaults": { @@ -2687,6 +2707,16 @@ }, "description": "The color used for indicating that a pull request is open." }, + { + "id": "pullRequests.statusOpenForeground", + "defaults": { + "dark": "#ffffff", + "light": "#000000", + "highContrast": "editor.foreground", + "highContrastLight": "editor.foreground" + }, + "description": "The foreground color used for the status badge indicating that a pull request is open." + }, { "id": "pullRequests.closed", "defaults": { @@ -2697,6 +2727,16 @@ }, "description": "The color used for indicating that a pull request is closed." }, + { + "id": "pullRequests.statusClosedForeground", + "defaults": { + "dark": "pullRequests.statusOpenForeground", + "light": "pullRequests.statusOpenForeground", + "highContrast": "editor.foreground", + "highContrastLight": "editor.foreground" + }, + "description": "The foreground color used for the status badge indicating that a pull request is closed." + }, { "id": "pullRequests.notification", "defaults": { diff --git a/webviews/editorWebview/index.css b/webviews/editorWebview/index.css index 44037dc6c..0e44debcc 100644 --- a/webviews/editorWebview/index.css +++ b/webviews/editorWebview/index.css @@ -463,7 +463,6 @@ body button .icon { #status { box-sizing: border-box; line-height: 18px; - color: var(--vscode-button-foreground); border-radius: 18px; padding: 4px 12px; margin-right: 10px; @@ -488,18 +487,22 @@ body button .icon { .status-badge-merged { background-color: var(--vscode-pullRequests-merged); + color: var(--vscode-pullRequests-statusMergedForeground); } .status-badge-open { background-color: var(--vscode-pullRequests-open); + color: var(--vscode-pullRequests-statusOpenForeground); } .status-badge-closed { background-color: var(--vscode-pullRequests-closed); + color: var(--vscode-pullRequests-statusClosedForeground); } .status-badge-draft { background-color: var(--vscode-pullRequests-draft); + color: var(--vscode-pullRequests-statusDraftForeground); } .section {