diff --git a/package.json b/package.json index 7dac45e809..408e9082c9 100644 --- a/package.json +++ b/package.json @@ -148,11 +148,6 @@ "id": "pr", "name": "GitHub Pull Requests", "when": "!config.githubPullRequests.showInSCM && config.git.enabled && github:hasGitHubRemotes && workspaceFolderCount != 0" - }, - { - "id": "prStatus", - "name": "Changes In Pull Request", - "when": "!config.githubPullRequests.showInSCM && config.git.enabled && github:hasGitHubRemotes && github:inReviewMode" } ], "scm": [ @@ -160,11 +155,6 @@ "id": "pr", "name": "GitHub Pull Requests", "when": "config.githubPullRequests.showInSCM && config.git.enabled && github:hasGitHubRemotes && workspaceFolderCount != 0" - }, - { - "id": "prStatus", - "name": "Changes In Pull Request", - "when": "config.githubPullRequests.showInSCM && config.git.enabled && github:hasGitHubRemotes && github:inReviewMode" } ] }, @@ -382,11 +372,6 @@ "command": "pr.refreshList", "when": "view == pr", "group": "navigation" - }, - { - "command": "pr.refreshChanges", - "when": "view == prStatus", - "group": "navigation" } ], "view/item/context": [ @@ -412,26 +397,26 @@ }, { "command": "pr.openFileInGitHub", - "when": "view =~ /(pr|prStatus)/ && viewItem =~ /filechange/" + "when": "view == pr && viewItem =~ /filechange/" }, { "command": "pr.copyCommitHash", - "when": "view == prStatus && viewItem =~ /commit/" + "when": "view == pr && viewItem =~ /commit/" }, { "command": "pr.openDescriptionToTheSide", "group": "inline", - "when": "view =~ /(pr|prStatus)/ && viewItem =~ /description/" + "when": "view == pr && viewItem =~ /description/" }, { "command": "review.openFile", "group": "inline", - "when": "config.git.openDiffOnClick && view == prStatus && viewItem =~ /filechange(?!:DELETE)/" + "when": "config.git.openDiffOnClick && view == pr && viewItem =~ /filechange:active(?!:DELETE)/" }, { "command": "pr.openDiffView", "group": "inline", - "when": "!config.git.openDiffOnClick && view == prStatus && viewItem =~ /filechange(?!:DELETE)/" + "when": "!config.git.openDiffOnClick && view == pr && viewItem =~ /filechange:active(?!:DELETE)/" } ], "editor/title": [ diff --git a/src/commands.ts b/src/commands.ts index dade8edd00..dcf524382b 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -25,6 +25,7 @@ import { GitErrorCodes } from './git/api'; import { Comment } from './common/comment'; import { PullRequestManager } from './github/pullRequestManager'; import { PullRequestModel } from './github/pullRequestModel'; +import { ActivePRNode } from './view/treeNodes/activePullRequestNode'; const _onDidUpdatePR = new vscode.EventEmitter(); export const onDidUpdatePR: vscode.Event = _onDidUpdatePR.event; @@ -258,9 +259,12 @@ export function registerCommands(context: vscode.ExtensionContext, prManager: Pu context.subscriptions.push(vscode.commands.registerCommand('pr.openDescription', async (descriptionNode: DescriptionNode) => { if (!descriptionNode) { - // the command is triggerred from command palette or status bar, which means we are already in checkout mode. - let rootNodes = await reviewManager.prFileChangesProvider.getChildren(); - descriptionNode = rootNodes[0] as DescriptionNode; + // the command is triggerred from command palette or status bar, which means we are already in checkout mode. Assume the PR exists + // in the "Local Pull Request Branches" category + const rootNodes = await reviewManager.prsTreeDataProvider.getChildren(); + const localFileChanges = await rootNodes[0].getChildren(); + const activePR = localFileChanges.filter(change => change instanceof ActivePRNode)[0]; + descriptionNode = (await activePR.getChildren())[0] as DescriptionNode; } const pullRequest = ensurePR(prManager, descriptionNode.pullRequestModel); // Create and show a new webview @@ -292,11 +296,10 @@ export function registerCommands(context: vscode.ExtensionContext, prManager: Pu } // Show the file change in a diff view. - let { path, ref, commit } = fromReviewUri(fileChange.filePath); + let { path, commit } = fromReviewUri(fileChange.filePath); let previousCommit = `${commit}^`; const query: ReviewUriParams = { path: path, - ref: ref, commit: previousCommit, base: true, isOutdated: true diff --git a/src/common/uri.ts b/src/common/uri.ts index f40bd259e6..7da3f04c9e 100644 --- a/src/common/uri.ts +++ b/src/common/uri.ts @@ -11,7 +11,6 @@ import { PullRequestModel } from '../github/pullRequestModel'; export interface ReviewUriParams { path: string; - ref?: string; commit?: string; base: boolean; isOutdated: boolean; @@ -43,10 +42,9 @@ export interface GitUriOptions { base: boolean; } -export function toDiffViewFileUri(uri: Uri, filePath: string | undefined, ref: string | undefined, commit: string, isOutdated: boolean, options: GitUriOptions): Uri { +export function toDiffViewFileUri(uri: Uri, filePath: string | undefined, commit: string, isOutdated: boolean, options: GitUriOptions): Uri { const params: ReviewUriParams = { path: filePath ? filePath : uri.path, - ref, commit: commit, base: options.base, isOutdated @@ -67,10 +65,9 @@ export function toDiffViewFileUri(uri: Uri, filePath: string | undefined, ref: s // As a mitigation for extensions like ESLint showing warnings and errors // for git URIs, let's change the file extension of these uris to .git, // when `replaceFileExtension` is true. -export function toReviewUri(uri: Uri, filePath: string | undefined, ref: string | undefined, commit: string, isOutdated: boolean, options: GitUriOptions): Uri { +export function toReviewUri(uri: Uri, filePath: string | undefined, commit: string, isOutdated: boolean, options: GitUriOptions): Uri { const params: ReviewUriParams = { path: filePath ? filePath : uri.path, - ref, commit: commit, base: options.base, isOutdated diff --git a/src/extension.ts b/src/extension.ts index c7922a3e9a..a077bb7a10 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -48,7 +48,7 @@ async function init(context: vscode.ExtensionContext, git: GitAPI, repository: R context.subscriptions.push(vscode.window.registerUriHandler(uriHandler)); context.subscriptions.push(new FileTypeDecorationProvider()); const prManager = new PullRequestManager(repository, telemetry); - const reviewManager = new ReviewManager(context, Keychain.onDidChange, repository, prManager, telemetry); + const reviewManager = new ReviewManager(Keychain.onDidChange, repository, prManager, telemetry); registerCommands(context, prManager, reviewManager, telemetry); git.repositories.forEach(repo => { diff --git a/src/github/pullRequestManager.ts b/src/github/pullRequestManager.ts index debb7d9b0b..0568c16e1b 100644 --- a/src/github/pullRequestManager.ts +++ b/src/github/pullRequestManager.ts @@ -22,6 +22,7 @@ import { EXTENSION_ID } from '../constants'; import { fromPRUri } from '../common/uri'; import { convertRESTPullRequestToRawPullRequest, convertPullRequestsGetCommentsResponseItemToComment, convertIssuesCreateCommentResponseToComment, parseGraphQLTimelineEvents, convertRESTTimelineEvents, getRelatedUsersFromTimelineEvents, parseGraphQLComment } from './utils'; import { PendingReviewIdResponse, TimelineEventsResponse, PullRequestCommentsResponse, AddCommentResponse, SubmitReviewResponse, DeleteReviewResponse, EditCommentResponse } from './graphql'; +import { GitFileChange } from '../view/treeNodes/fileChangeNode'; const queries = require('./queries.gql'); interface PageInformation { @@ -98,6 +99,9 @@ export const onDidSubmitReview: vscode.Event = _onDidSubmitReview.eve export class PullRequestManager { static ID = 'PullRequestManager'; private _activePullRequest?: PullRequestModel; + public activeFileChanges?: GitFileChange[]; + public activeOutdatedFileChanges?: GitFileChange[]; + public activeComments?: Comment[]; private _credentialStore: CredentialStore; private _githubRepositories: GitHubRepository[]; private _mentionableUsers?: { [key: string]: IAccount[] }; diff --git a/src/view/prChangesTreeDataProvider.ts b/src/view/prChangesTreeDataProvider.ts deleted file mode 100644 index 6e31bc9a54..0000000000 --- a/src/view/prChangesTreeDataProvider.ts +++ /dev/null @@ -1,140 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * 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 { GitFileChangeNode, RemoteFileChangeNode } from './treeNodes/fileChangeNode'; -import { DescriptionNode } from './treeNodes/descriptionNode'; -import { TreeNode } from './treeNodes/treeNode'; -import { FilesCategoryNode } from './treeNodes/filesCategoryNode'; -import { CommitsNode } from './treeNodes/commitsCategoryNode'; -import { Comment } from '../common/comment'; -import { PullRequestManager } from '../github/pullRequestManager'; -import { PullRequestModel } from '../github/pullRequestModel'; - -export class PullRequestChangesTreeDataProvider extends vscode.Disposable implements vscode.TreeDataProvider { - private _onDidChangeTreeData = new vscode.EventEmitter(); - readonly onDidChangeTreeData = this._onDidChangeTreeData.event; - private _disposables: vscode.Disposable[] = []; - - private _localFileChanges: (GitFileChangeNode | RemoteFileChangeNode)[] = []; - private _comments: Comment[] = []; - private _pullrequest?: PullRequestModel; - private _pullRequestManager: PullRequestManager; - private _view: vscode.TreeView; - - public get view(): vscode.TreeView { - return this._view; - } - - private _descriptionNode?: DescriptionNode; - private _filesCategoryNode?: FilesCategoryNode; - private _commitsCategoryNode?: CommitsNode; - - constructor(private _context: vscode.ExtensionContext) { - super(() => this.dispose()); - this._view = vscode.window.createTreeView('prStatus', { - treeDataProvider: this, - showCollapseAll: true - }); - this._context.subscriptions.push(this._view); - } - - refresh() { - this._descriptionNode = undefined; - this._filesCategoryNode = undefined; - this._commitsCategoryNode = undefined; - this._onDidChangeTreeData.fire(); - } - - async showPullRequestFileChanges(pullRequestManager: PullRequestManager, pullrequest: PullRequestModel, fileChanges: (GitFileChangeNode | RemoteFileChangeNode)[], comments: Comment[]) { - this._pullRequestManager = pullRequestManager; - this._pullrequest = pullrequest; - this._comments = comments; - - await vscode.commands.executeCommand( - 'setContext', - 'github:inReviewMode', - true - ); - - this._localFileChanges = fileChanges; - this._descriptionNode = undefined; - this._filesCategoryNode = undefined; - this._commitsCategoryNode = undefined; - this._onDidChangeTreeData.fire(); - } - - async hide() { - await vscode.commands.executeCommand( - 'setContext', - 'github:inReviewMode', - false - ); - } - - getTreeItem(element: TreeNode): vscode.TreeItem | Thenable { - return element.getTreeItem(); - } - - getParent(element: TreeNode) { - return element.getParent(); - } - - async reveal(element: TreeNode, options?: { select?: boolean, focus?: boolean, expand?: boolean | number }): Promise { - this._view.reveal(element, options); - } - - async revealComment(comment: Comment) { - let fileChange = this._localFileChanges.find(fc => { - if (fc.fileName !== comment.path) { - return false; - } - - if (fc.pullRequest.head.sha !== comment.commitId) { - return false; - } - - return true; - }); - - if (fileChange) { - await this.reveal(fileChange, { focus: true, expand: 2 }); - if (!fileChange.command.arguments) { - return; - } - if (fileChange instanceof GitFileChangeNode) { - let lineNumber = fileChange.getCommentPosition(comment); - const opts = fileChange.opts; - opts.selection = new vscode.Range(lineNumber, 0, lineNumber, 0); - fileChange.opts = opts; - await vscode.commands.executeCommand(fileChange.command.command, fileChange); - } else { - await vscode.commands.executeCommand(fileChange.command.command, ...fileChange.command.arguments!); - } - } - } - - async getChildren(element?: GitFileChangeNode): Promise { - if (!this._pullrequest) { - return []; - } - - if (!element) { - if (!this._descriptionNode || !this._filesCategoryNode || !this._commitsCategoryNode) { - this._descriptionNode = new DescriptionNode(this, this._pullrequest.title, - this._pullrequest.userAvatarUri!, this._pullrequest); - this._filesCategoryNode = new FilesCategoryNode(this._view, this._localFileChanges); - this._commitsCategoryNode = new CommitsNode(this._view, this._pullRequestManager, this._pullrequest, this._comments); - } - return [ this._descriptionNode, this._filesCategoryNode, this._commitsCategoryNode ]; - } else { - return await element.getChildren(); - } - } - - dispose() { - this._disposables.forEach(disposable => disposable.dispose()); - } -} \ No newline at end of file diff --git a/src/view/prsTreeDataProvider.ts b/src/view/prsTreeDataProvider.ts index 014b0f479a..cd72ed48e5 100644 --- a/src/view/prsTreeDataProvider.ts +++ b/src/view/prsTreeDataProvider.ts @@ -19,6 +19,7 @@ export class PullRequestsTreeDataProvider implements vscode.TreeDataProvider; + private _firstLoad = true; get view(): vscode.TreeView { return this._view; @@ -68,14 +69,15 @@ export class PullRequestsTreeDataProvider implements vscode.TreeDataProvider(); -function workspaceLocalCommentsToCommentThreads(repository: Repository, fileChange: GitFileChangeNode, fileComments: Comment[], collapsibleState: vscode.CommentThreadCollapsibleState): vscode.CommentThread[] { +function workspaceLocalCommentsToCommentThreads(repository: Repository, fileChange: GitFileChange, fileComments: Comment[], collapsibleState: vscode.CommentThreadCollapsibleState): vscode.CommentThread[] { if (!fileChange) { return []; } @@ -77,8 +77,8 @@ export class ReviewDocumentCommentProvider implements vscode.DocumentCommentProv constructor( private _prManager: PullRequestManager, private _repository: Repository, - private _localFileChanges: GitFileChangeNode[], - private _obsoleteFileChanges: (GitFileChangeNode | RemoteFileChangeNode)[], + private _localFileChanges: GitFileChange[], + private _obsoleteFileChanges: GitFileChange[], private _comments: Comment[]) { const supportsGraphQL = _prManager.activePullRequest!.githubRepository.supportsGraphQl; if (supportsGraphQL) { @@ -195,8 +195,8 @@ export class ReviewDocumentCommentProvider implements vscode.DocumentCommentProv // local file, we only provide active comments // TODO. for comments in deleted ranges, they should show on top of the first line. const fileName = nodePath.relative(currentWorkspace!.uri.fsPath, document.uri.fsPath); - const matchedFiles = gitFileChangeNodeFilter(this._localFileChanges).filter(fileChange => fileChange.fileName === fileName); - let matchedFile: GitFileChangeNode; + const matchedFiles = this._localFileChanges.filter(fileChange => fileChange.fileName === fileName); + let matchedFile: GitFileChange; let matchingComments: Comment[] = []; let ranges = []; @@ -240,7 +240,7 @@ export class ReviewDocumentCommentProvider implements vscode.DocumentCommentProv }; } - private findMatchedFileChangeForReviewDiffView(fileChanges: (GitFileChangeNode | RemoteFileChangeNode)[], uri: vscode.Uri): GitFileChangeNode | undefined { + private findMatchedFileChangeForReviewDiffView(fileChanges: (GitFileChange | RemoteFileChangeNode)[], uri: vscode.Uri): GitFileChangeNode | undefined { let query = fromReviewUri(uri); let matchedFiles = fileChanges.filter(fileChange => { if (fileChange instanceof RemoteFileChangeNode) { @@ -275,7 +275,7 @@ export class ReviewDocumentCommentProvider implements vscode.DocumentCommentProv } } - private findMatchedFileByUri(document: vscode.TextDocument): GitFileChangeNode | undefined { + private findMatchedFileByUri(document: vscode.TextDocument): GitFileChange | undefined { const uri = document.uri; let fileName: string; @@ -295,7 +295,7 @@ export class ReviewDocumentCommentProvider implements vscode.DocumentCommentProv } const fileChangesToSearch = isOutdated ? this._obsoleteFileChanges : this._localFileChanges; - const matchedFiles = gitFileChangeNodeFilter(fileChangesToSearch).filter(fileChange => { + const matchedFiles = fileChangesToSearch.filter(fileChange => { if (uri.scheme === 'review' || uri.scheme === 'pr') { return fileChange.fileName === fileName; } else { @@ -673,7 +673,7 @@ export class ReviewDocumentCommentProvider implements vscode.DocumentCommentProv for (let file in fileCommentGroups) { let fileComments: Comment[] = fileCommentGroups[file]; - let matchedFiles = gitFileChangeNodeFilter(this._localFileChanges).filter(fileChange => fileChange.fileName === file); + let matchedFiles = this._localFileChanges.filter(fileChange => fileChange.fileName === file); if (matchedFiles && matchedFiles.length) { ret = [...ret, ...workspaceLocalCommentsToCommentThreads(this._repository, matchedFiles[0], fileComments, collapsibleState)]; @@ -726,23 +726,23 @@ export class ReviewDocumentCommentProvider implements vscode.DocumentCommentProv export class ReviewWorkspaceCommentsPRovider implements vscode.WorkspaceCommentProvider { constructor( private _repository: Repository, - private _localFileChanges: GitFileChangeNode[], - private _obsoleteFileChanges: (GitFileChangeNode | RemoteFileChangeNode)[]) { + private _localFileChanges: GitFileChange[], + private _obsoleteFileChanges: GitFileChange[]) { } onDidChangeCommentThreads = _onDidChangeWorkspaceCommentThreads.event; async provideWorkspaceComments(token: vscode.CancellationToken) { - const comments = await Promise.all(gitFileChangeNodeFilter(this._localFileChanges).map(async fileChange => { + const comments = await Promise.all(this._localFileChanges.map(async fileChange => { return workspaceLocalCommentsToCommentThreads(this._repository, fileChange, fileChange.comments, vscode.CommentThreadCollapsibleState.Expanded); })); - const outdatedComments = gitFileChangeNodeFilter(this._obsoleteFileChanges).map(fileChange => { + const outdatedComments = this._obsoleteFileChanges.map(fileChange => { return this.outdatedCommentsToCommentThreads(fileChange, fileChange.comments, vscode.CommentThreadCollapsibleState.Expanded); }); return [...comments, ...outdatedComments].reduce((prev, curr) => prev.concat(curr), []); } - private outdatedCommentsToCommentThreads(fileChange: GitFileChangeNode, fileComments: Comment[], collapsibleState: vscode.CommentThreadCollapsibleState): vscode.CommentThread[] { + private outdatedCommentsToCommentThreads(fileChange: GitFileChange, fileComments: Comment[], collapsibleState: vscode.CommentThreadCollapsibleState): vscode.CommentThread[] { if (!fileComments || !fileComments.length) { return []; } diff --git a/src/view/reviewManager.ts b/src/view/reviewManager.ts index a3ff0fd340..9de29c9538 100644 --- a/src/view/reviewManager.ts +++ b/src/view/reviewManager.ts @@ -12,10 +12,9 @@ import { Comment } from '../common/comment'; import { GitChangeType, InMemFileChange, SlimFileChange } from '../common/file'; import { ITelemetry } from '../github/interface'; import { Repository, GitErrorCodes, Branch } from '../git/api'; -import { PullRequestChangesTreeDataProvider } from './prChangesTreeDataProvider'; import { GitContentProvider } from './gitContentProvider'; import { DiffChangeType } from '../common/diffHunk'; -import { GitFileChangeNode, RemoteFileChangeNode, gitFileChangeNodeFilter } from './treeNodes/fileChangeNode'; +import { GitFileChange, GitFileChangeNode } from './treeNodes/fileChangeNode'; import Logger from '../common/logger'; import { PullRequestsTreeDataProvider } from './prsTreeDataProvider'; import { PRNode } from './treeNodes/pullRequestNode'; @@ -33,15 +32,12 @@ export class ReviewManager implements vscode.DecorationProvider { private _disposables: vscode.Disposable[]; private _comments: Comment[] = []; - private _localFileChanges: (GitFileChangeNode)[] = []; - private _obsoleteFileChanges: (GitFileChangeNode | RemoteFileChangeNode)[] = []; private _lastCommitSha?: string; private _updateMessageShown: boolean = false; private _validateStatusInProgress?: Promise; private _reviewDocumentCommentProvider: ReviewDocumentCommentProvider; - private _prsTreeDataProvider: PullRequestsTreeDataProvider; - private _prFileChangesProvider: PullRequestChangesTreeDataProvider; + public prsTreeDataProvider: PullRequestsTreeDataProvider; private _statusBarItem: vscode.StatusBarItem; private _prNumber?: number; private _previousRepositoryState: { @@ -63,7 +59,6 @@ export class ReviewManager implements vscode.DecorationProvider { } constructor( - private _context: vscode.ExtensionContext, onShouldReload: vscode.Event, private _repository: Repository, private _prManager: PullRequestManager, @@ -78,8 +73,8 @@ export class ReviewManager implements vscode.DecorationProvider { this.registerCommands(); this.registerListeners(); - this._prsTreeDataProvider = new PullRequestsTreeDataProvider(onShouldReload, _prManager, this._telemetry); - this._disposables.push(this._prsTreeDataProvider); + this.prsTreeDataProvider = new PullRequestsTreeDataProvider(onShouldReload, _prManager, this._telemetry); + this._disposables.push(this.prsTreeDataProvider); this._disposables.push(vscode.window.registerDecorationProvider(this)); this._previousRepositoryState = { @@ -129,7 +124,6 @@ export class ReviewManager implements vscode.DecorationProvider { this._disposables.push(vscode.commands.registerCommand('pr.refreshChanges', _ => { this.updateComments(); PullRequestOverviewPanel.refresh(); - this.prFileChangesProvider.refresh(); })); this._disposables.push(vscode.commands.registerCommand('pr.refreshPullRequest', (prNode: PRNode) => { @@ -138,7 +132,7 @@ export class ReviewManager implements vscode.DecorationProvider { } PullRequestOverviewPanel.refresh(); - this._prsTreeDataProvider.refresh(prNode); + this.prsTreeDataProvider.refresh(prNode); })); } @@ -189,15 +183,6 @@ export class ReviewManager implements vscode.DecorationProvider { return ReviewManager._instance; } - get prFileChangesProvider() { - if (!this._prFileChangesProvider) { - this._prFileChangesProvider = new PullRequestChangesTreeDataProvider(this._context); - this._disposables.push(this._prFileChangesProvider); - } - - return this._prFileChangesProvider; - } - get statusBarItem() { if (!this._statusBarItem) { this._statusBarItem = vscode.window.createStatusBarItem(vscode.StatusBarAlignment.Left); @@ -280,7 +265,6 @@ export class ReviewManager implements vscode.DecorationProvider { this._lastCommitSha = pr.head.sha; await this.getPullRequestData(pr); - await this.prFileChangesProvider.showPullRequestFileChanges(this._prManager, pr, this._localFileChanges, this._comments); this._onDidChangeDecorations.fire(); Logger.appendLine(`Review> register comments provider`); @@ -331,8 +315,8 @@ export class ReviewManager implements vscode.DecorationProvider { return Promise.resolve(void 0); } - private async getLocalChangeNodes(pr: PullRequestModel, contentChanges: (InMemFileChange | SlimFileChange)[], activeComments: Comment[]): Promise { - let nodes: GitFileChangeNode[] = []; + private async getLocalChangeNodes(pr: PullRequestModel, contentChanges: (InMemFileChange | SlimFileChange)[], activeComments: Comment[]): Promise { + let nodes: GitFileChange[] = []; const mergeBase = pr.mergeBase || pr.base.sha; const headSha = pr.head.sha; @@ -355,16 +339,15 @@ export class ReviewManager implements vscode.DecorationProvider { const filePath = nodePath.resolve(this._repository.rootUri.fsPath, change.fileName); const uri = vscode.Uri.file(filePath); - let changedItem = new GitFileChangeNode( - this.prFileChangesProvider.view, + let changedItem = new GitFileChange( pr, change.status, change.fileName, change.blobUrl, change.status === GitChangeType.DELETE ? - toReviewUri(uri, undefined, undefined, '', false, { base: false }) : - toDiffViewFileUri(uri, change.fileName, undefined, pr.head.sha, false, { base: false }), - toReviewUri(uri, change.fileName, undefined, change.status === GitChangeType.ADD ? '' : mergeBase, false, { base: true }), + toReviewUri(uri, undefined, '', false, { base: false }) : + toDiffViewFileUri(uri, change.fileName, pr.head.sha, false, { base: false }), + toReviewUri(uri, change.fileName, change.status === GitChangeType.ADD ? '' : mergeBase, false, { base: true }), isPartial, diffHunks, activeComments.filter(comment => comment.path === change.fileName), @@ -379,6 +362,7 @@ export class ReviewManager implements vscode.DecorationProvider { private async getPullRequestData(pr: PullRequestModel): Promise { try { this._comments = await this._prManager.getPullRequestComments(pr); + this._prManager.activeComments = this._comments; let activeComments = this._comments.filter(comment => comment.position); let outdatedComments = this._comments.filter(comment => !comment.position); @@ -386,10 +370,10 @@ export class ReviewManager implements vscode.DecorationProvider { const mergeBase = pr.mergeBase || pr.base.sha; const contentChanges = await parseDiff(data, this._repository, mergeBase!); - this._localFileChanges = await this.getLocalChangeNodes(pr, contentChanges, activeComments); + this._prManager.activeFileChanges = await this.getLocalChangeNodes(pr, contentChanges, activeComments); let commitsGroup = groupBy(outdatedComments, comment => comment.originalCommitId!); - this._obsoleteFileChanges = []; + const obsoleteFileChanges = []; for (let commit in commitsGroup) { let commentsForCommit = commitsGroup[commit]; let commentsForFile = groupBy(commentsForCommit, comment => comment.path!); @@ -406,24 +390,25 @@ export class ReviewManager implements vscode.DecorationProvider { const oldComments = commentsForFile[fileName]; const uri = vscode.Uri.parse(nodePath.join(`commit~${commit.substr(0, 8)}`, fileName)); - const obsoleteFileChange = new GitFileChangeNode( - this.prFileChangesProvider.view, + const obsoleteFileChange = new GitFileChange( pr, GitChangeType.MODIFY, fileName, undefined, - toReviewUri(uri, fileName, undefined, oldComments[0].originalCommitId!, true, { base: false }), - toReviewUri(uri, fileName, undefined, oldComments[0].originalCommitId!, true, { base: true }), + toReviewUri(uri, fileName, oldComments[0].originalCommitId!, true, { base: false }), + toReviewUri(uri, fileName, oldComments[0].originalCommitId!, true, { base: true }), false, diffHunks, oldComments, commit ); - this._obsoleteFileChanges.push(obsoleteFileChange); + obsoleteFileChanges.push(obsoleteFileChange); } } + this._prManager.activeOutdatedFileChanges = obsoleteFileChanges; + return Promise.resolve(void 0); } catch (e) { Logger.appendLine(`Review> ${e}`); @@ -455,8 +440,8 @@ export class ReviewManager implements vscode.DecorationProvider { private registerCommentProvider() { this._reviewDocumentCommentProvider = new ReviewDocumentCommentProvider(this._prManager, this._repository, - this._localFileChanges, - this._obsoleteFileChanges, + this._prManager.activeFileChanges!, + this._prManager.activeOutdatedFileChanges!, this._comments); this._localToDispose.push(this._reviewDocumentCommentProvider); @@ -470,8 +455,8 @@ export class ReviewManager implements vscode.DecorationProvider { this._localToDispose.push(vscode.workspace.registerWorkspaceCommentProvider(new ReviewWorkspaceCommentsPRovider( this._repository, - this._localFileChanges, - this._obsoleteFileChanges))); + this._prManager.activeFileChanges!, + this._prManager.activeOutdatedFileChanges!))); } public async switch(pr: PullRequestModel): Promise { @@ -706,10 +691,6 @@ export class ReviewManager implements vscode.DecorationProvider { this._statusBarItem.hide(); } - if (this._prFileChangesProvider) { - this.prFileChangesProvider.hide(); - } - // Ensure file explorer decorations are removed. When switching to a different PR branch, // comments are recalculated when getting the data and the change decoration fired then, // so comments only needs to be emptied in this case. @@ -722,9 +703,9 @@ export class ReviewManager implements vscode.DecorationProvider { async provideTextDocumentContent(uri: vscode.Uri): Promise { let { path, commit } = fromReviewUri(uri); - let changedItems = gitFileChangeNodeFilter(this._localFileChanges) + let changedItems = (this._prManager.activeFileChanges || []) .filter(change => change.fileName === path) - .filter(fileChange => fileChange.sha === commit || (fileChange.parentSha ? fileChange.parentSha : `${fileChange.sha}^`) === commit); + .filter(fileChange => fileChange.sha === commit || `${fileChange.sha}^` === commit); if (changedItems.length) { let changedItem = changedItems[0]; @@ -733,9 +714,9 @@ export class ReviewManager implements vscode.DecorationProvider { return ret.reduce((prev, curr) => prev.concat(...curr), []).join('\n'); } - changedItems = gitFileChangeNodeFilter(this._obsoleteFileChanges) + changedItems = (this._prManager.activeOutdatedFileChanges || []) .filter(change => change.fileName === path) - .filter(fileChange => fileChange.sha === commit || (fileChange.parentSha ? fileChange.parentSha : `${fileChange.sha}^`) === commit); + .filter(fileChange => fileChange.sha === commit || `${fileChange.sha}^` === commit); if (changedItems.length) { // it's from obsolete file changes, which means the content is in complete. diff --git a/src/view/treeNodes/activePullRequestNode.ts b/src/view/treeNodes/activePullRequestNode.ts new file mode 100644 index 0000000000..e7ef1710ec --- /dev/null +++ b/src/view/treeNodes/activePullRequestNode.ts @@ -0,0 +1,95 @@ +/*--------------------------------------------------------------------------------------------- + * 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 { TreeNode } from './treeNode'; +import { CommitsNode } from './commitsCategoryNode'; +import { PullRequestManager } from '../../github/pullRequestManager'; +import { DescriptionNode } from './descriptionNode'; +import { Resource } from '../../common/resources'; +import { Comment } from '../../common/comment'; +import { GitFileChangeNode } from './fileChangeNode'; + +export class ActivePRNode extends TreeNode { + private _pullRequestManager: PullRequestManager; + private _isLocal: boolean; + private _fileNodes: GitFileChangeNode[]; + + constructor(parent: TreeNode, prManager: PullRequestManager, local: boolean) { + super(); + this.parent = parent; + this._pullRequestManager = prManager; + this._isLocal = local; + this._fileNodes = (this._pullRequestManager.activeFileChanges || []).map(change => new GitFileChangeNode( + this, + this._pullRequestManager.activePullRequest!, + change.status, change.fileName, change.blobUrl, change.filePath, + change.parentFilePath, change.isPartial, change.diffHunks, change.comments, + change.sha)); + } + + getTreeItem(): vscode.TreeItem { + const { + title, + prNumber, + author, + userAvatarUri + } = this._pullRequestManager.activePullRequest!; + + const { login } = author; + + const formattedPRNumber = prNumber.toString(); + const label = `✓ ${title}`; + const tooltip = `Current Branch * ${title} (#${formattedPRNumber}) by @${login}`; + const description = `#${formattedPRNumber} by @${login}`; + + return { + label, + tooltip, + description, + collapsibleState: vscode.TreeItemCollapsibleState.Expanded, + contextValue: 'pullrequest' + (this._isLocal ? ':local' : '') + ':active', + iconPath: userAvatarUri + }; + } + + async getChildren(): Promise { + return [ + new DescriptionNode(this, 'Description', { + light: Resource.icons.light.Description, + dark: Resource.icons.dark.Description + }, this._pullRequestManager.activePullRequest!), + ...this._fileNodes, + new CommitsNode(this, this._pullRequestManager, this._pullRequestManager.activePullRequest!, this._pullRequestManager.activeComments!) + ]; + } + + async revealComment(comment: Comment) { + const fileChange = this._fileNodes.find(fc => { + if (fc.fileName !== comment.path) { + return false; + } + + if (fc.pullRequest.head.sha !== comment.commitId) { + return false; + } + + return true; + }); + + if (fileChange) { + await this.reveal(fileChange, { focus: true, expand: 2 }); + if (!fileChange.command.arguments) { + return; + } + + const lineNumber = fileChange.getCommentPosition(comment); + const opts = fileChange.opts; + opts.selection = new vscode.Range(lineNumber, 0, lineNumber, 0); + fileChange.opts = opts; + await vscode.commands.executeCommand(fileChange.command.command, fileChange); + } + } +} \ No newline at end of file diff --git a/src/view/treeNodes/categoryNode.ts b/src/view/treeNodes/categoryNode.ts index 41efe8191a..b58297c750 100644 --- a/src/view/treeNodes/categoryNode.ts +++ b/src/view/treeNodes/categoryNode.ts @@ -11,6 +11,7 @@ import { formatError } from '../../common/utils'; import { AuthenticationError } from '../../common/authentication'; import { PullRequestManager } from '../../github/pullRequestManager'; import { PullRequestModel } from '../../github/pullRequestModel'; +import { ActivePRNode } from './activePullRequestNode'; export enum PRCategoryActionType { Empty, @@ -89,12 +90,15 @@ export class CategoryTreeNode extends TreeNode implements vscode.TreeItem { public parent: TreeNode | vscode.TreeView, private _prManager: PullRequestManager, private _telemetry: ITelemetry, - private _type: PRType + private _type: PRType, + firstLoad: boolean ) { super(); this.prs = []; - this.collapsibleState = vscode.TreeItemCollapsibleState.Collapsed; + this.collapsibleState = firstLoad && !!_prManager.activePullRequest && _type === PRType.LocalPullRequest + ? vscode.TreeItemCollapsibleState.Expanded + : vscode.TreeItemCollapsibleState.Collapsed; switch (_type) { case PRType.All: this.label = 'All'; @@ -171,7 +175,10 @@ export class CategoryTreeNode extends TreeNode implements vscode.TreeItem { } if (this.prs && this.prs.length) { - let nodes: TreeNode[] = this.prs.map(prItem => new PRNode(this, this._prManager, prItem, this._type === PRType.LocalPullRequest)); + let nodes: TreeNode[] = this.prs.map(prItem => + prItem.equals(this._prManager.activePullRequest) + ? new ActivePRNode(this, this._prManager, this._type === PRType.LocalPullRequest) + : new PRNode(this, this._prManager, prItem, this._type === PRType.LocalPullRequest)); if (hasMorePages) { nodes.push(new PRCategoryActionNode(this, PRCategoryActionType.More, this)); } else if (hasUnsearchedRepositories) { diff --git a/src/view/treeNodes/commitNode.ts b/src/view/treeNodes/commitNode.ts index d80b4c184d..4b1c982a30 100644 --- a/src/view/treeNodes/commitNode.ts +++ b/src/view/treeNodes/commitNode.ts @@ -50,6 +50,17 @@ export class CommitNode extends TreeNode implements vscode.TreeItem { } async getChildren(): Promise { + const currentlyCheckedOut = this.pullRequest.equals(this.pullRequestManager.activePullRequest); + + if (!currentlyCheckedOut) { + vscode.window.showInformationMessage('To view commits, this pull request must be checked out.', 'Checkout').then(result => { + if (result === 'Checkout') { + vscode.commands.executeCommand('pr.pick', this.pullRequest); + } + }); + + return []; + } const fileChanges = await this.pullRequestManager.getCommitChangedFiles(this.pullRequest, this.commit); const fileChangeNodes = fileChanges.map(change => { @@ -62,8 +73,8 @@ export class CommitNode extends TreeNode implements vscode.TreeItem { getGitChangeType(change.status), fileName, undefined, - toReviewUri(uri, fileName, undefined, this.commit.sha, true, { base: false }), - toReviewUri(uri, fileName, undefined, this.commit.sha, true, { base: true }), + toReviewUri(uri, fileName, this.commit.sha, true, { base: false }), + toReviewUri(uri, fileName, this.commit.sha, true, { base: true }), false, [], matchingComments, diff --git a/src/view/treeNodes/fileChangeNode.ts b/src/view/treeNodes/fileChangeNode.ts index f6b06865c1..0f9fba87cb 100644 --- a/src/view/treeNodes/fileChangeNode.ts +++ b/src/view/treeNodes/fileChangeNode.ts @@ -131,6 +131,21 @@ export class InMemFileChangeNode extends TreeNode implements vscode.TreeItem { } } +export class GitFileChange { + constructor( + public readonly pullRequest: PullRequestModel, + public readonly status: GitChangeType, + public readonly fileName: string, + public readonly blobUrl: string | undefined, + public readonly filePath: vscode.Uri, + public readonly parentFilePath: vscode.Uri, + public readonly isPartial: boolean, + public readonly diffHunks: DiffHunk[], + public comments: Comment[] = [], + public readonly sha?: string, + ) { } +} + /** * File change node whose content can be resolved by git commit sha. */ @@ -145,7 +160,7 @@ export class GitFileChangeNode extends TreeNode implements vscode.TreeItem { public opts: vscode.TextDocumentShowOptions; constructor( - public readonly parent: TreeNode | vscode.TreeView, + public readonly parent: TreeNode, public readonly pullRequest: PullRequestModel, public readonly status: GitChangeType, public readonly fileName: string, @@ -158,7 +173,7 @@ export class GitFileChangeNode extends TreeNode implements vscode.TreeItem { public readonly sha?: string, ) { super(); - this.contextValue = `filechange:${GitChangeType[status]}`; + this.contextValue = `filechange:active:${GitChangeType[status]}`; this.label = path.basename(fileName); this.description = path.relative('.', path.dirname(fileName)); this.iconPath = vscode.ThemeIcon.File; diff --git a/src/view/treeNodes/pullRequestNode.ts b/src/view/treeNodes/pullRequestNode.ts index 528cd8dec8..2c723b1bd4 100644 --- a/src/view/treeNodes/pullRequestNode.ts +++ b/src/view/treeNodes/pullRequestNode.ts @@ -13,18 +13,19 @@ import { Resource } from '../../common/resources'; import { fromPRUri, toPRUri } from '../../common/uri'; import { groupBy, formatError } from '../../common/utils'; import { DescriptionNode } from './descriptionNode'; -import { RemoteFileChangeNode, InMemFileChangeNode, GitFileChangeNode } from './fileChangeNode'; +import { RemoteFileChangeNode, InMemFileChangeNode, GitFileChange } from './fileChangeNode'; import { TreeNode } from './treeNode'; import { getInMemPRContentProvider } from '../inMemPRContentProvider'; import { Comment } from '../../common/comment'; import { PullRequestManager, onDidSubmitReview } from '../../github/pullRequestManager'; import { PullRequestModel } from '../../github/pullRequestModel'; import { convertToVSCodeComment } from '../../github/utils'; +import { CommitsNode } from './commitsCategoryNode'; export function providePRDocumentComments( document: vscode.TextDocument, prNumber: number, - fileChanges: (RemoteFileChangeNode | InMemFileChangeNode | GitFileChangeNode)[], + fileChanges: (RemoteFileChangeNode | InMemFileChangeNode | GitFileChange)[], inDraftMode: boolean) { const params = fromPRUri(document.uri); @@ -281,40 +282,39 @@ export class PRNode extends TreeNode { this._inMemPRContentProvider = getInMemPRContentProvider().registerTextDocumentContentProvider(this.pullRequestModel.prNumber, this.provideDocumentContent.bind(this)); } - // The review manager will register a document comment's provider, so the node does not need to - if (!this.pullRequestModel.equals(this._prManager.activePullRequest)) { - if (this._documentCommentsProvider) { - // diff comments - await this.updateComments(comments, fileChanges); - this._fileChanges = fileChanges; - } else { - this._fileChanges = fileChanges; - this._onDidChangeCommentThreads = new vscode.EventEmitter(); - await this.pullRequestModel.githubRepository.ensureCommentsProvider(); - this._documentCommentsProvider = this.pullRequestModel.githubRepository.commentsProvider.registerDocumentCommentProvider(this.pullRequestModel, { - onDidChangeCommentThreads: this._onDidChangeCommentThreads.event, - provideDocumentComments: this.provideDocumentComments.bind(this), - createNewCommentThread: this.createNewCommentThread.bind(this), - replyToCommentThread: this.replyToCommentThread.bind(this), - editComment: this.editComment.bind(this), - deleteComment: this.deleteComment.bind(this), - startDraft: this.startDraft.bind(this), - finishDraft: this.finishDraft.bind(this), - deleteDraft: this.deleteDraft.bind(this) - }); - - this._disposables.push(onDidSubmitReview(_ => { - this.updateCommentPendingState(); - })); - } + if (this._documentCommentsProvider) { + // diff comments + await this.updateComments(comments, fileChanges); + this._fileChanges = fileChanges; } else { this._fileChanges = fileChanges; + this._onDidChangeCommentThreads = new vscode.EventEmitter(); + await this.pullRequestModel.githubRepository.ensureCommentsProvider(); + this._documentCommentsProvider = this.pullRequestModel.githubRepository.commentsProvider.registerDocumentCommentProvider(this.pullRequestModel, { + onDidChangeCommentThreads: this._onDidChangeCommentThreads.event, + provideDocumentComments: this.provideDocumentComments.bind(this), + createNewCommentThread: this.createNewCommentThread.bind(this), + replyToCommentThread: this.replyToCommentThread.bind(this), + editComment: this.editComment.bind(this), + deleteComment: this.deleteComment.bind(this), + startDraft: this.startDraft.bind(this), + finishDraft: this.finishDraft.bind(this), + deleteDraft: this.deleteDraft.bind(this) + }); + + this._disposables.push(onDidSubmitReview(_ => { + this.updateCommentPendingState(); + })); } - let result = [new DescriptionNode(this, 'Description', { - light: Resource.icons.light.Description, - dark: Resource.icons.dark.Description - }, this.pullRequestModel), ...this._fileChanges]; + let result = [ + new DescriptionNode(this, 'Description', { + light: Resource.icons.light.Description, + dark: Resource.icons.dark.Description + }, this.pullRequestModel), + ...this._fileChanges, + new CommitsNode(this, this._prManager, this.pullRequestModel, comments) + ]; this.childrenDisposables = result; return result; @@ -355,8 +355,6 @@ export class PRNode extends TreeNode { } getTreeItem(): vscode.TreeItem { - const currentBranchIsForThisPR = this.pullRequestModel.equals(this._prManager.activePullRequest); - const { title, prNumber, @@ -367,11 +365,9 @@ export class PRNode extends TreeNode { login, } = author; - const labelPrefix = (currentBranchIsForThisPR ? '✓ ' : ''); - const tooltipPrefix = (currentBranchIsForThisPR ? 'Current Branch * ' : ''); const formattedPRNumber = prNumber.toString(); - const label = `${labelPrefix}${title}`; - const tooltip = `${tooltipPrefix}${title} (#${formattedPRNumber}) by @${login}`; + const label = title; + const tooltip = `${title} (#${formattedPRNumber}) by @${login}`; const description = `#${formattedPRNumber} by @${login}`; return { @@ -379,7 +375,7 @@ export class PRNode extends TreeNode { tooltip, description, collapsibleState: 1, - contextValue: 'pullrequest' + (this._isLocal ? ':local' : '') + (currentBranchIsForThisPR ? ':active' : ':nonactive'), + contextValue: 'pullrequest' + (this._isLocal ? ':local' : '') + ':nonactive', iconPath: this.pullRequestModel.userAvatarUri }; }