#457 New "View Diff with Working File" action on the file context men…

…u in the Commit Details View.

src/gitGraphView.ts

@@ -8,7 +8,7 @@ import { Logger } from './logger';
 import { RepoFileWatcher } from './repoFileWatcher';
 import { RepoManager } from './repoManager';
 import { ErrorInfo, GitConfigLocation, GitGraphViewInitialState, GitPushBranchMode, GitRepoSet, LoadGitGraphViewTo, RequestMessage, ResponseMessage, TabIconColourTheme } from './types';
-import { UNABLE_TO_FIND_GIT_MSG, UNCOMMITTED, archive, copyFilePathToClipboard, copyToClipboard, createPullRequest, getNonce, openExtensionSettings, openExternalUrl, openFile, showErrorMessage, viewDiff, viewFileAtRevision, viewScm } from './utils';
+import { UNABLE_TO_FIND_GIT_MSG, UNCOMMITTED, archive, copyFilePathToClipboard, copyToClipboard, createPullRequest, getNonce, openExtensionSettings, openExternalUrl, openFile, showErrorMessage, viewDiff, viewDiffWithWorkingFile, viewFileAtRevision, viewScm } from './utils';
 import { Disposable, toDisposable } from './utils/disposable';
 
 /**
@@ -582,6 +582,12 @@ export class GitGraphView extends Disposable {
 					error: await viewDiff(msg.repo, msg.fromHash, msg.toHash, msg.oldFilePath, msg.newFilePath, msg.type)
 				});
 				break;
+			case 'viewDiffWithWorkingFile':
+				this.sendMessage({
+					command: 'viewDiffWithWorkingFile',
+					error: await viewDiffWithWorkingFile(msg.repo, msg.hash, msg.filePath)
+				});
+				break;
 			case 'viewFileAtRevision':
 				this.sendMessage({
 					command: 'viewFileAtRevision',

src/types.ts

@@ -1172,6 +1172,15 @@ export interface ResponseViewDiff extends ResponseWithErrorInfo {
 	readonly command: 'viewDiff';
 }
 
+export interface RequestViewDiffWithWorkingFile extends RepoRequest {
+	readonly command: 'viewDiffWithWorkingFile';
+	readonly hash: string;
+	readonly filePath: string;
+}
+export interface ResponseViewDiffWithWorkingFile extends ResponseWithErrorInfo {
+	readonly command: 'viewDiffWithWorkingFile';
+}
+
 export interface RequestViewFileAtRevision extends RepoRequest {
 	readonly command: 'viewFileAtRevision';
 	readonly hash: string;
@@ -1247,6 +1256,7 @@ export type RequestMessage =
 	| RequestStartCodeReview
 	| RequestTagDetails
 	| RequestViewDiff
+	| RequestViewDiffWithWorkingFile
 	| RequestViewFileAtRevision
 	| RequestViewScm;
 
@@ -1305,6 +1315,7 @@ export type ResponseMessage =
 	| ResponseStartCodeReview
 	| ResponseTagDetails
 	| ResponseViewDiff
+	| ResponseViewDiffWithWorkingFile
 	| ResponseViewFileAtRevision
 	| ResponseViewScm;
 

src/utils.ts

@@ -381,13 +381,29 @@ export function viewDiff(repo: string, fromHash: string, toHash: string, oldFile
 			viewColumn: getConfig().openNewTabEditorGroup
 		}).then(
 			() => null,
-			() => 'Visual Studio Code was unable load the diff editor for ' + newFilePath + '.'
+			() => 'Visual Studio Code was unable to load the diff editor for ' + newFilePath + '.'
 		);
 	} else {
 		return openFile(repo, newFilePath);
 	}
 }
 
+/**
+ * Open the Visual Studio Code Diff View to display the changes of a file between a commit hash and the working tree.
+ * @param repo The repository the file is contained in.
+ * @param hash The revision of the left-side of the Diff View.
+ * @param filePath The relative path of the file within the repository.
+ * @returns A promise resolving to the ErrorInfo of the executed command.
+ */
+export function viewDiffWithWorkingFile(repo: string, hash: string, filePath: string) {
+	return new Promise<ErrorInfo>((resolve) => {
+		const p = path.join(repo, filePath);
+		fs.access(p, fs.constants.R_OK, (err) => {
+			resolve(viewDiff(repo, hash, UNCOMMITTED, filePath, filePath, err === null ? GitFileStatus.Modified : GitFileStatus.Deleted));
+		});
+	});
+}
+
 /**
  * Open a Visual Studio Code Editor (readonly) for a file a specific Git revision.
  * @param repo The repository the file is contained in.

tests/utils.test.ts

@@ -24,7 +24,7 @@ import { DataSource } from '../src/dataSource';
 import { ExtensionState } from '../src/extensionState';
 import { Logger } from '../src/logger';
 import { GitFileStatus, PullRequestProvider } from '../src/types';
-import { GitExecutable, UNCOMMITTED, abbrevCommit, abbrevText, archive, constructIncompatibleGitVersionMessage, copyFilePathToClipboard, copyToClipboard, createPullRequest, evalPromises, findGit, getExtensionVersion, getGitExecutable, getGitExecutableFromPaths, getNonce, getPathFromStr, getPathFromUri, getRelativeTimeDiff, getRepoName, isGitAtLeastVersion, isPathInWorkspace, openExtensionSettings, openExternalUrl, openFile, openGitTerminal, pathWithTrailingSlash, realpath, resolveSpawnOutput, resolveToSymbolicPath, showErrorMessage, showInformationMessage, viewDiff, viewFileAtRevision, viewScm } from '../src/utils';
+import { GitExecutable, UNCOMMITTED, abbrevCommit, abbrevText, archive, constructIncompatibleGitVersionMessage, copyFilePathToClipboard, copyToClipboard, createPullRequest, evalPromises, findGit, getExtensionVersion, getGitExecutable, getGitExecutableFromPaths, getNonce, getPathFromStr, getPathFromUri, getRelativeTimeDiff, getRepoName, isGitAtLeastVersion, isPathInWorkspace, openExtensionSettings, openExternalUrl, openFile, openGitTerminal, pathWithTrailingSlash, realpath, resolveSpawnOutput, resolveToSymbolicPath, showErrorMessage, showInformationMessage, viewDiff, viewDiffWithWorkingFile, viewFileAtRevision, viewScm } from '../src/utils';
 import { EventEmitter } from '../src/utils/event';
 
 const extensionContext = vscode.mocks.extensionContext;
@@ -1232,7 +1232,7 @@ describe('viewDiff', () => {
 		const result = await viewDiff('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f', 'subfolder/modified.txt', 'subfolder/modified.txt', GitFileStatus.Modified);
 
 		// Assert
-		expect(result).toBe('Visual Studio Code was unable load the diff editor for subfolder/modified.txt.');
+		expect(result).toBe('Visual Studio Code was unable to load the diff editor for subfolder/modified.txt.');
 	});
 
 	it('Should open an untracked file in vscode', async () => {
@@ -1255,6 +1255,62 @@ describe('viewDiff', () => {
 	});
 });
 
+describe('viewDiffWithWorkingFile', () => {
+	it('Should load the vscode diff view (modified file)', async () => {
+		// Setup
+		mockedFileSystemModule.access.mockImplementationOnce((_1: fs.PathLike, _2: number | undefined, callback: (err: NodeJS.ErrnoException | null) => void) => callback(null));
+		vscode.commands.executeCommand.mockResolvedValueOnce(null);
+
+		// Run
+		const result = await viewDiffWithWorkingFile('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f', 'subfolder/modified.txt');
+
+		// Assert
+		const [command, leftUri, rightUri, title, config] = vscode.commands.executeCommand.mock.calls[0];
+		expect(command).toBe('vscode.diff');
+		expect(leftUri.toString()).toBe('git-graph://file.txt?eyJmaWxlUGF0aCI6InN1YmZvbGRlci9tb2RpZmllZC50eHQiLCJjb21taXQiOiIxYTJiM2M0ZDVlNmYxYTJiM2M0ZDVlNmYxYTJiM2M0ZDVlNmYxYTJiM2M0ZDVlNmYiLCJyZXBvIjoiL3BhdGgvdG8vcmVwbyJ9');
+		expect(getPathFromUri(rightUri)).toBe('/path/to/repo/subfolder/modified.txt');
+		expect(title).toBe('modified.txt (1a2b3c4d ↔ Present)');
+		expect(config).toStrictEqual({
+			preview: true,
+			viewColumn: vscode.ViewColumn.Active
+		});
+		expect(result).toBe(null);
+	});
+
+	it('Should load the vscode diff view (deleted file)', async () => {
+		// Setup
+		mockedFileSystemModule.access.mockImplementationOnce((_1: fs.PathLike, _2: number | undefined, callback: (err: NodeJS.ErrnoException | null) => void) => callback(new Error()));
+		vscode.commands.executeCommand.mockResolvedValueOnce(null);
+
+		// Run
+		const result = await viewDiffWithWorkingFile('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f', 'subfolder/deleted.txt');
+
+		// Assert
+		const [command, leftUri, rightUri, title, config] = vscode.commands.executeCommand.mock.calls[0];
+		expect(command).toBe('vscode.diff');
+		expect(leftUri.toString()).toBe('git-graph://file.txt?eyJmaWxlUGF0aCI6InN1YmZvbGRlci9kZWxldGVkLnR4dCIsImNvbW1pdCI6IjFhMmIzYzRkNWU2ZjFhMmIzYzRkNWU2ZjFhMmIzYzRkNWU2ZjFhMmIzYzRkNWU2ZiIsInJlcG8iOiIvcGF0aC90by9yZXBvIn0=');
+		expect(rightUri.toString()).toBe('git-graph://file?bnVsbA==');
+		expect(title).toBe('deleted.txt (Deleted between 1a2b3c4d & Present)');
+		expect(config).toStrictEqual({
+			preview: true,
+			viewColumn: vscode.ViewColumn.Active
+		});
+		expect(result).toBe(null);
+	});
+
+	it('Should return an error message when vscode was unable to load the diff view', async () => {
+		// Setup
+		mockedFileSystemModule.access.mockImplementationOnce((_1: fs.PathLike, _2: number | undefined, callback: (err: NodeJS.ErrnoException | null) => void) => callback(null));
+		vscode.commands.executeCommand.mockRejectedValueOnce(null);
+
+		// Run
+		const result = await viewDiffWithWorkingFile('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f', 'subfolder/modified.txt');
+
+		// Assert
+		expect(result).toBe('Visual Studio Code was unable to load the diff editor for subfolder/modified.txt.');
+	});
+});
+
 describe('viewFileAtRevision', () => {
 	it('Should open the file in vscode', async () => {
 		// Setup

web/main.ts

@@ -2714,7 +2714,7 @@ class GitGraphView {
 		});
 	}
 
-	private cdvFileViewed(filePath: string, fileElem: HTMLElement) {
+	private cdvFileViewed(filePath: string, fileElem: HTMLElement, markAsCodeReviewed: boolean) {
 		const expandedCommit = this.expandedCommit, filesElem = document.getElementById('cdvFiles');
 		if (expandedCommit === null || expandedCommit.fileTree === null || filesElem === null) return;
 
@@ -2727,7 +2727,7 @@ class GitGraphView {
 		lastViewedElem.innerHTML = SVG_ICONS.eyeOpen;
 		insertBeforeFirstChildWithClass(lastViewedElem, fileElem, 'fileTreeFileAction');
 
-		if (expandedCommit.codeReview !== null) {
+		if (expandedCommit.codeReview !== null && markAsCodeReviewed) {
 			let i = expandedCommit.codeReview.remainingFiles.indexOf(filePath);
 			if (i > -1) {
 				sendMessage({ command: 'codeReviewFileReviewed', repo: this.currentRepo, id: expandedCommit.codeReview.id, filePath: filePath });
@@ -2785,6 +2785,17 @@ class GitGraphView {
 		const getFileElemOfEventTarget = (target: EventTarget) => <HTMLElement>(<Element>target).closest('.fileTreeFileRecord');
 		const getFileOfFileElem = (fileChanges: ReadonlyArray<GG.GitFileChange>, fileElem: HTMLElement) => fileChanges[parseInt(fileElem.dataset.index!)];
 
+		const getCommitHashForFile = (file: GG.GitFileChange, expandedCommit: ExpandedCommit) => {
+			const commit = this.commits[this.commitLookup[expandedCommit.commitHash]];
+			if (expandedCommit.compareWithHash !== null) {
+				return this.getCommitOrder(expandedCommit.commitHash, expandedCommit.compareWithHash).to;
+			} else if (commit.stash !== null && file.type === GG.GitFileStatus.Untracked) {
+				return commit.stash.untrackedFilesHash!;
+			} else {
+				return expandedCommit.commitHash;
+			}
+		};
+
 		const triggerViewFileDiff = (file: GG.GitFileChange, fileElem: HTMLElement) => {
 			const expandedCommit = this.expandedCommit;
 			if (expandedCommit === null) return;
@@ -2811,7 +2822,7 @@ class GitGraphView {
 				toHash = expandedCommit.commitHash;
 			}
 
-			this.cdvFileViewed(file.newFilePath, fileElem);
+			this.cdvFileViewed(file.newFilePath, fileElem, true);
 			sendMessage({
 				command: 'viewDiff',
 				repo: this.currentRepo,
@@ -2831,21 +2842,20 @@ class GitGraphView {
 			const expandedCommit = this.expandedCommit;
 			if (expandedCommit === null) return;
 
-			let commit = this.commits[this.commitLookup[expandedCommit.commitHash]], hash: string;
-			if (expandedCommit.compareWithHash !== null) {
-				hash = this.getCommitOrder(expandedCommit.commitHash, expandedCommit.compareWithHash).to;
-			} else if (commit.stash !== null && file.type === GG.GitFileStatus.Untracked) {
-				hash = commit.stash.untrackedFilesHash!;
-			} else {
-				hash = expandedCommit.commitHash;
-			}
+			this.cdvFileViewed(file.newFilePath, fileElem, true);
+			sendMessage({ command: 'viewFileAtRevision', repo: this.currentRepo, hash: getCommitHashForFile(file, expandedCommit), filePath: file.newFilePath });
+		};
+
+		const triggerViewFileDiffWithWorkingFile = (file: GG.GitFileChange, fileElem: HTMLElement) => {
+			const expandedCommit = this.expandedCommit;
+			if (expandedCommit === null) return;
 
-			this.cdvFileViewed(file.newFilePath, fileElem);
-			sendMessage({ command: 'viewFileAtRevision', repo: this.currentRepo, hash: hash, filePath: file.newFilePath });
+			this.cdvFileViewed(file.newFilePath, fileElem, false);
+			sendMessage({ command: 'viewDiffWithWorkingFile', repo: this.currentRepo, hash: getCommitHashForFile(file, expandedCommit), filePath: file.newFilePath });
 		};
 
 		const triggerOpenFile = (file: GG.GitFileChange, fileElem: HTMLElement) => {
-			this.cdvFileViewed(file.newFilePath, fileElem);
+			this.cdvFileViewed(file.newFilePath, fileElem, true);
 			sendMessage({ command: 'openFile', repo: this.currentRepo, filePath: file.newFilePath });
 		};
 
@@ -2922,6 +2932,7 @@ class GitGraphView {
 				elem: fileElem
 			};
 			const diffPossible = file.type === GG.GitFileStatus.Untracked || (file.additions !== null && file.deletions !== null);
+			const fileExistsAtThisRevisionAndDiffPossible = file.type !== GG.GitFileStatus.Deleted && diffPossible && !isUncommitted;
 
 			contextMenu.show([
 				[
@@ -2932,9 +2943,14 @@ class GitGraphView {
 					},
 					{
 						title: 'View File at this Revision',
-						visible: file.type !== GG.GitFileStatus.Deleted && diffPossible && !isUncommitted,
+						visible: fileExistsAtThisRevisionAndDiffPossible,
 						onClick: () => triggerViewFileAtRevision(file, fileElem)
 					},
+					{
+						title: 'View Diff with Working File',
+						visible: fileExistsAtThisRevisionAndDiffPossible,
+						onClick: () => triggerViewFileDiffWithWorkingFile(file, fileElem)
+					},
 					{
 						title: 'Open File',
 						visible: file.type !== GG.GitFileStatus.Deleted,
@@ -3240,7 +3256,10 @@ window.addEventListener('load', () => {
 				}
 				break;
 			case 'viewDiff':
-				finishOrDisplayError(msg.error, 'Unable to View Diff of File');
+				finishOrDisplayError(msg.error, 'Unable to View Diff');
+				break;
+			case 'viewDiffWithWorkingFile':
+				finishOrDisplayError(msg.error, 'Unable to View Diff with Working File');
 				break;
 			case 'viewFileAtRevision':
 				finishOrDisplayError(msg.error, 'Unable to View File at Revision');