Add "Mark as viewed/unviewed" to editor toolbar (#3420)

package.json

@@ -1394,6 +1394,16 @@
 					"command": "issue.createIssueFromFile",
 					"group": "navigation",
 					"when": "resourceFilename == NewIssue.md"
+				},
+				{
+					"command": "pr.markFileAsViewed",
+					"group": "navigation",
+					"when": "resourceScheme != pr && resourceScheme != review && resourceScheme != filechange && resourcePath in github:unviewedFiles"
+				},
+				{
+					"command": "pr.unmarkFileAsViewed",
+					"group": "navigation",
+					"when": "resourceScheme != pr && resourceScheme != review && resourceScheme != filechange && resourcePath in github:viewedFiles"
 				}
 			],
 			"scm/title": [

src/commands.ts

@@ -30,6 +30,7 @@ import { CategoryTreeNode } from './view/treeNodes/categoryNode';
 import { CommitNode } from './view/treeNodes/commitNode';
 import { DescriptionNode } from './view/treeNodes/descriptionNode';
 import {
+	FileChangeNode,
 	GitFileChangeNode,
 	InMemFileChangeNode,
 	openFileCommand,
@@ -853,19 +854,39 @@ export function registerCommands(
 	);
 
 	context.subscriptions.push(
-		vscode.commands.registerCommand('pr.markFileAsViewed', async (treeNode: GitFileChangeNode) => {
+		vscode.commands.registerCommand('pr.markFileAsViewed', async (treeNode: FileChangeNode | vscode.Uri) => {
 			try {
-				await treeNode.pullRequest.markFileAsViewed(treeNode.fileName);
+				if (treeNode instanceof FileChangeNode) {
+					await treeNode.pullRequest.markFileAsViewed(treeNode.fileName);
+					const manager = reposManager.getManagerForFile(treeNode.filePath);
+					if (treeNode.pullRequest === manager?.activePullRequest) {
+						treeNode.pullRequest.setFileViewedContext();
+					}
+				} else {
+					const manager = reposManager.getManagerForFile(treeNode);
+					await manager?.activePullRequest?.markFileAsViewed(treeNode.path);
+					manager?.activePullRequest?.setFileViewedContext();
+				}
 			} catch (e) {
 				vscode.window.showErrorMessage(`Marked file as viewed failed: ${e}`);
 			}
 		}),
 	);
 
 	context.subscriptions.push(
-		vscode.commands.registerCommand('pr.unmarkFileAsViewed', async (treeNode: GitFileChangeNode) => {
+		vscode.commands.registerCommand('pr.unmarkFileAsViewed', async (treeNode: FileChangeNode | vscode.Uri) => {
 			try {
-				await treeNode.pullRequest.unmarkFileAsViewed(treeNode.fileName);
+				if (treeNode instanceof FileChangeNode) {
+					await treeNode.pullRequest.unmarkFileAsViewed(treeNode.fileName);
+					const manager = reposManager.getManagerForFile(treeNode.filePath);
+					if (treeNode.pullRequest === manager?.activePullRequest) {
+						treeNode.pullRequest.setFileViewedContext();
+					}
+				} else {
+					const manager = reposManager.getManagerForFile(treeNode);
+					await manager?.activePullRequest?.unmarkFileAsViewed(treeNode.path);
+					manager?.activePullRequest?.setFileViewedContext();
+				}
 			} catch (e) {
 				vscode.window.showErrorMessage(`Marked file as not viewed failed: ${e}`);
 			}

src/common/executeCommands.ts

@@ -5,12 +5,21 @@
 
 import * as vscode from 'vscode';
 
+export namespace contexts {
+	export const VIEWED_FILES = 'github:viewedFiles';
+	export const UNVIEWED_FILES = 'github:unviewedFiles';
+}
+
 export namespace commands {
-	export function executeCommand(command: string) {
-		return vscode.commands.executeCommand(command);
+	export function executeCommand(command: string, arg1?: any, arg2?: any) {
+		return vscode.commands.executeCommand(command, arg1, arg2);
 	}
 
 	export function focusView(viewId: string) {
 		return executeCommand(`${viewId}.focus`);
 	}
+
+	export function setContext(context: string, value: any) {
+		return executeCommand('setContext', context, value);
+	}
 }

src/github/folderRepositoryManager.ts

@@ -1894,7 +1894,7 @@ export class FolderRepositoryManager implements vscode.Disposable {
 	}
 
 	private createAndAddGitHubRepository(remote: Remote, credentialStore: CredentialStore) {
-		const repo = new GitHubRepository(remote, credentialStore, this.telemetry, this._sessionState);
+		const repo = new GitHubRepository(remote, this.repository.rootUri, credentialStore, this.telemetry, this._sessionState);
 		this._githubRepositories.push(repo);
 		return repo;
 	}

src/github/githubRepository.ts

@@ -148,6 +148,7 @@ export class GitHubRepository implements vscode.Disposable {
 
 	constructor(
 		public remote: Remote,
+		public readonly rootUri: vscode.Uri,
 		private readonly _credentialStore: CredentialStore,
 		private readonly _telemetry: ITelemetry,
 		private readonly _sessionState: ISessionState
@@ -410,7 +411,7 @@ export class GitHubRepository implements vscode.Disposable {
 				parsedIssue.repositoryUrl,
 				new Protocol(parsedIssue.repositoryUrl),
 			);
-			githubRepository = new GitHubRepository(remote, this._credentialStore, this._telemetry, this._sessionState);
+			githubRepository = new GitHubRepository(remote, this.rootUri, this._credentialStore, this._telemetry, this._sessionState);
 		}
 		return githubRepository;
 	}

src/github/pullRequestModel.ts

@@ -10,6 +10,7 @@ import * as vscode from 'vscode';
 import { Repository } from '../api/api';
 import { DiffSide, IComment, IReviewThread, ViewedState } from '../common/comment';
 import { parseDiff } from '../common/diffHunk';
+import { commands, contexts } from '../common/executeCommands';
 import { GitChangeType, InMemFileChange, SlimFileChange } from '../common/file';
 import { GitHubRef } from '../common/githubRef';
 import Logger from '../common/logger';
@@ -107,6 +108,8 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
 	public onDidChangeReviewThreads = this._onDidChangeReviewThreads.event;
 
 	private _fileChangeViewedState: FileViewedState = {};
+	private _viewedFiles: Set<string> = new Set();
+	private _unviewedFiles: Set<string> = new Set();
 	private _onDidChangeFileViewedState = new vscode.EventEmitter<FileViewedStateChangeEvent>();
 	public onDidChangeFileViewedState = this._onDidChangeFileViewedState.event;
 
@@ -1388,8 +1391,9 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
 				if (this._fileChangeViewedState[n.path] !== n.viewerViewedState) {
 					changed.push({ fileName: n.path, viewed: n.viewerViewedState });
 				}
-
-				this._fileChangeViewedState[n.path] = n.viewerViewedState;
+				// No event for setting the file viewed state here.
+				// Instead, wait until all the changes have been made and set the context at the end.
+				this.setFileViewedState(n.path, n.viewerViewedState, false);
 			});
 
 			hasNextPage = data.repository.pullRequest.files.pageInfo.hasNextPage;
@@ -1401,8 +1405,10 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
 		}
 	}
 
-	async markFileAsViewed(fileName: string): Promise<void> {
+	async markFileAsViewed(filePathOrSubpath: string): Promise<void> {
 		const { mutate, schema } = await this.githubRepository.ensure();
+		const fileName = filePathOrSubpath.startsWith(this.githubRepository.rootUri.path) ?
+			filePathOrSubpath.substring(this.githubRepository.rootUri.path.length + 1) : filePathOrSubpath;
 		await mutate<void>({
 			mutation: schema.MarkFileAsViewed,
 			variables: {
@@ -1413,12 +1419,13 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
 			},
 		});
 
-		this._fileChangeViewedState[fileName] = ViewedState.VIEWED;
-		this._onDidChangeFileViewedState.fire({ changed: [{ fileName, viewed: ViewedState.VIEWED }] });
+		this.setFileViewedState(fileName, ViewedState.VIEWED, true);
 	}
 
-	async unmarkFileAsViewed(fileName: string): Promise<void> {
+	async unmarkFileAsViewed(filePathOrSubpath: string): Promise<void> {
 		const { mutate, schema } = await this.githubRepository.ensure();
+		const fileName = filePathOrSubpath.startsWith(this.githubRepository.rootUri.path) ?
+			filePathOrSubpath.substring(this.githubRepository.rootUri.path.length + 1) : filePathOrSubpath;
 		await mutate<void>({
 			mutation: schema.UnmarkFileAsViewed,
 			variables: {
@@ -1429,7 +1436,39 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
 			},
 		});
 
-		this._fileChangeViewedState[fileName] = ViewedState.UNVIEWED;
-		this._onDidChangeFileViewedState.fire({ changed: [{ fileName, viewed: ViewedState.UNVIEWED }] });
+		this.setFileViewedState(fileName, ViewedState.UNVIEWED, true);
+	}
+
+	private setFileViewedState(fileSubpath: string, viewedState: ViewedState, event: boolean) {
+		const filePath = vscode.Uri.joinPath(this.githubRepository.rootUri, fileSubpath).fsPath;
+		switch (viewedState) {
+			case ViewedState.DISMISSED: {
+				this._viewedFiles.delete(filePath);
+				this._unviewedFiles.delete(filePath);
+				break;
+			}
+			case ViewedState.UNVIEWED: {
+				this._viewedFiles.delete(filePath);
+				this._unviewedFiles.add(filePath);
+				break;
+			}
+			case ViewedState.VIEWED: {
+				this._viewedFiles.add(filePath);
+				this._unviewedFiles.delete(filePath);
+			}
+		}
+		this._fileChangeViewedState[fileSubpath] = viewedState;
+		if (event) {
+			this._onDidChangeFileViewedState.fire({ changed: [{ fileName: fileSubpath, viewed: viewedState }] });
+		}
+	}
+
+	/**
+	 * Using these contexts is fragile in a multi-root workspace where multiple PRs are checked out.
+	 * If you have two active PRs that have the same file path relative to their rootdir, then these context can get confused.
+	 */
+	public setFileViewedContext() {
+		commands.setContext(contexts.VIEWED_FILES, Array.from(this._viewedFiles));
+		commands.setContext(contexts.UNVIEWED_FILES, Array.from(this._unviewedFiles));
 	}
 }

src/test/github/folderRepositoryManager.test.ts

@@ -15,6 +15,7 @@ import { GitApiImpl } from '../../api/api1';
 import { CredentialStore } from '../../github/credentials';
 import { MockExtensionContext } from '../mocks/mockExtensionContext';
 import { MockSessionState } from '../mocks/mockSessionState';
+import { Uri } from 'vscode';
 
 describe('PullRequestManager', function () {
 	let sinon: SinonSandbox;
@@ -46,7 +47,8 @@ describe('PullRequestManager', function () {
 			const url = 'https://github.com/aaa/bbb.git';
 			const protocol = new Protocol(url);
 			const remote = new Remote('origin', url, protocol);
-			const repository = new GitHubRepository(remote, manager.credentialStore, telemetry, new MockSessionState());
+			const rootUri = Uri.file('C:\\users\\test\\repo');
+			const repository = new GitHubRepository(remote, rootUri, manager.credentialStore, telemetry, new MockSessionState());
 			const prItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().build(), repository);
 			const pr = new PullRequestModel(telemetry, repository, remote, prItem);
 

src/test/github/githubRepository.test.ts

@@ -7,6 +7,7 @@ import { Remote } from '../../common/remote';
 import { Protocol } from '../../common/protocol';
 import { GitHubRepository } from '../../github/githubRepository';
 import { MockSessionState } from '../mocks/mockSessionState';
+import { Uri } from 'vscode';
 
 describe('GitHubRepository', function () {
 	let sinon: SinonSandbox;
@@ -29,14 +30,16 @@ describe('GitHubRepository', function () {
 		it('detects when the remote is pointing to github.com', function () {
 			const url = 'https://github.com/some/repo';
 			const remote = new Remote('origin', url, new Protocol(url));
-			const dotcomRepository = new GitHubRepository(remote, credentialStore, telemetry, new MockSessionState());
+			const rootUri = Uri.file('C:\\users\\test\\repo');
+			const dotcomRepository = new GitHubRepository(remote, rootUri, credentialStore, telemetry, new MockSessionState());
 			assert(dotcomRepository.isGitHubDotCom);
 		});
 
 		it('detects when the remote is pointing somewhere other than github.com', function () {
 			const url = 'https://github.enterprise.horse/some/repo';
 			const remote = new Remote('origin', url, new Protocol(url));
-			const dotcomRepository = new GitHubRepository(remote, credentialStore, telemetry, new MockSessionState());
+			const rootUri = Uri.file('C:\\users\\test\\repo');
+			const dotcomRepository = new GitHubRepository(remote, rootUri, credentialStore, telemetry, new MockSessionState());
 			assert(!dotcomRepository.isGitHubDotCom);
 		});
 	});

src/test/mocks/mockGitHubRepository.ts

@@ -14,13 +14,14 @@ import {
 } from '../builders/managedPullRequestBuilder';
 import { MockTelemetry } from './mockTelemetry';
 import { MockSessionState } from './mockSessionState';
+import { Uri } from 'vscode';
 const queries = require('../../github/queries.gql');
 
 export class MockGitHubRepository extends GitHubRepository {
 	readonly queryProvider: QueryProvider;
 
 	constructor(remote: Remote, credentialStore: CredentialStore, telemetry: MockTelemetry, sinon: SinonSandbox) {
-		super(remote, credentialStore, telemetry, new MockSessionState());
+		super(remote, Uri.file('C:\\users\\test\\repo'),credentialStore, telemetry, new MockSessionState());
 
 		this.queryProvider = new QueryProvider(sinon);
 

src/view/reviewManager.ts

@@ -523,6 +523,7 @@ export class ReviewManager {
 			const contentChanges = await pr.getFileChangesInfo(this._repository);
 			this._reviewModel.localFileChanges = await this.getLocalChangeNodes(pr, contentChanges);
 			await Promise.all([pr.initializeReviewComments(), pr.initializeReviewThreadCache(), pr.initializePullRequestFileViewState()]);
+			pr.setFileViewedContext();
 			const outdatedComments = pr.comments.filter(comment => !comment.position);
 
 			const commitsGroup = groupBy(outdatedComments, comment => comment.originalCommitId!);