Add "Mark as Un/reviewed" options to files in code review mode

Fixes mhutchie#482

src/commands.ts

@@ -324,7 +324,7 @@ export class CommandManager extends Disposable {
 		if (typeof uri === 'object' && uri.scheme === DiffDocProvider.scheme) {
 			// A Git Graph URI has been provided
 			const request = decodeDiffDocUri(uri);
-			return openFile(request.repo, request.filePath, vscode.ViewColumn.Active).then((errorInfo) => {
+			return openFile(request.repo, request.filePath, request.commit, this.dataSource, vscode.ViewColumn.Active).then((errorInfo) => {
 				if (errorInfo !== null) {
 					return showErrorMessage('Unable to Open File: ' + errorInfo);
 				}

src/dataSource.ts

@@ -473,6 +473,20 @@ export class DataSource extends Disposable {
 		}).then((url) => url, () => null);
 	}
 
+	/**
+	 * Check to see if a file has been renamed between a commit and the working tree, and return the new file path.
+	 * @param repo The path of the repository.
+	 * @param commitHash The commit hash where `oldFilePath` is known to have existed.
+	 * @param oldFilePath The file path that may have been renamed.
+	 * @returns The new renamed file path, or NULL if either: the file wasn't renamed or the Git command failed to execute.
+	 */
+	public getNewPathOfRenamedFile(repo: string, commitHash: string, oldFilePath: string) {
+		return this.getDiffNameStatus(repo, commitHash, '', 'R').then((renamed) => {
+			const renamedRecordForFile = renamed.find((record) => record.oldFilePath === oldFilePath);
+			return renamedRecordForFile ? renamedRecordForFile.newFilePath : null;
+		}).catch(() => null);
+	}
+
 	/**
 	 * Get the details of a tag.
 	 * @param repo The path of the repository.
@@ -1386,10 +1400,11 @@ export class DataSource extends Disposable {
 	 * @param repo The path of the repository.
 	 * @param fromHash The revision the diff is from.
 	 * @param toHash The revision the diff is to.
+	 * @param filter The types of file changes to retrieve (defaults to `AMDR`).
 	 * @returns An array of `--name-status` records.
 	 */
-	private getDiffNameStatus(repo: string, fromHash: string, toHash: string) {
-		return this.execDiff(repo, fromHash, toHash, '--name-status').then((output) => {
+	private getDiffNameStatus(repo: string, fromHash: string, toHash: string, filter: string = 'AMDR') {
+		return this.execDiff(repo, fromHash, toHash, '--name-status', filter).then((output) => {
 			let records: DiffNameStatusRecord[] = [], i = 0;
 			while (i < output.length && output[i] !== '') {
 				let type = <GitFileStatus>output[i][0];
@@ -1415,10 +1430,11 @@ export class DataSource extends Disposable {
 	 * @param repo The path of the repository.
 	 * @param fromHash The revision the diff is from.
 	 * @param toHash The revision the diff is to.
+	 * @param filter The types of file changes to retrieve (defaults to `AMDR`).
 	 * @returns An array of `--numstat` records.
 	 */
-	private getDiffNumStat(repo: string, fromHash: string, toHash: string) {
-		return this.execDiff(repo, fromHash, toHash, '--numstat').then((output) => {
+	private getDiffNumStat(repo: string, fromHash: string, toHash: string, filter: string = 'AMDR') {
+		return this.execDiff(repo, fromHash, toHash, '--numstat', filter).then((output) => {
 			let records: DiffNumStatRecord[] = [], i = 0;
 			while (i < output.length && output[i] !== '') {
 				let fields = output[i].split('\t');
@@ -1656,14 +1672,15 @@ export class DataSource extends Disposable {
 	 * @param fromHash The revision the diff is from.
 	 * @param toHash The revision the diff is to.
 	 * @param arg Sets the data reported from the diff.
+	 * @param filter The types of file changes to retrieve.
 	 * @returns The diff output.
 	 */
-	private execDiff(repo: string, fromHash: string, toHash: string, arg: '--numstat' | '--name-status') {
+	private execDiff(repo: string, fromHash: string, toHash: string, arg: '--numstat' | '--name-status', filter: string) {
 		let args: string[];
 		if (fromHash === toHash) {
-			args = ['diff-tree', arg, '-r', '--root', '--find-renames', '--diff-filter=AMDR', '-z', fromHash];
+			args = ['diff-tree', arg, '-r', '--root', '--find-renames', '--diff-filter=' + filter, '-z', fromHash];
 		} else {
-			args = ['diff', arg, '--find-renames', '--diff-filter=AMDR', '-z', fromHash];
+			args = ['diff', arg, '--find-renames', '--diff-filter=' + filter, '-z', fromHash];
 			if (toHash !== '') args.push(toHash);
 		}
 

src/gitGraphView.ts

@@ -464,7 +464,7 @@ export class GitGraphView extends Disposable {
 			case 'openFile':
 				this.sendMessage({
 					command: 'openFile',
-					error: await openFile(msg.repo, msg.filePath)
+					error: await openFile(msg.repo, msg.filePath, msg.hash, this.dataSource)
 				});
 				break;
 			case 'openTerminal':
@@ -589,7 +589,7 @@ export class GitGraphView extends Disposable {
 			case 'viewDiffWithWorkingFile':
 				this.sendMessage({
 					command: 'viewDiffWithWorkingFile',
-					error: await viewDiffWithWorkingFile(msg.repo, msg.hash, msg.filePath)
+					error: await viewDiffWithWorkingFile(msg.repo, msg.hash, msg.filePath, this.dataSource)
 				});
 				break;
 			case 'viewFileAtRevision':

src/types.ts

@@ -972,6 +972,7 @@ export interface ResponseOpenExternalUrl extends ResponseWithErrorInfo {
 
 export interface RequestOpenFile extends RepoRequest {
 	readonly command: 'openFile';
+	readonly hash: string;
 	readonly filePath: string;
 }
 export interface ResponseOpenFile extends ResponseWithErrorInfo {

src/utils.ts

@@ -105,6 +105,17 @@ export async function resolveToSymbolicPath(path: string) {
 	return path;
 }
 
+/**
+ * Checks whether a file exists, and the user has access to read it.
+ * @param path The path of the file.
+ * @returns Promise resolving to a boolean: TRUE => File exists, FALSE => File doesn't exist.
+ */
+export function doesFileExist(path: string) {
+	return new Promise<boolean>((resolve) => {
+		fs.access(path, fs.constants.R_OK, (err) => resolve(err === null));
+	});
+}
+
 
 /* General Methods */
 
@@ -334,26 +345,38 @@ export function openExternalUrl(url: string, type: string = 'External URL'): The
  * Open a file within a repository in Visual Studio Code.
  * @param repo The repository the file is contained in.
  * @param filePath The relative path of the file within the repository.
+ * @param hash An optional commit hash where the file is known to have existed.
+ * @param dataSource An optional DataSource instance, that's used to check if the file has been renamed.
  * @param viewColumn An optional ViewColumn that the file should be opened in.
  * @returns A promise resolving to the ErrorInfo of the executed command.
  */
-export function openFile(repo: string, filePath: string, viewColumn: vscode.ViewColumn | null = null) {
-	return new Promise<ErrorInfo>(resolve => {
-		const p = path.join(repo, filePath);
-		fs.access(p, fs.constants.R_OK, (err) => {
-			if (err === null) {
-				vscode.commands.executeCommand('vscode.open', vscode.Uri.file(p), {
-					preview: true,
-					viewColumn: viewColumn === null ? getConfig().openNewTabEditorGroup : viewColumn
-				}).then(
-					() => resolve(null),
-					() => resolve('Visual Studio Code was unable to open ' + filePath + '.')
-				);
-			} else {
-				resolve('The file ' + filePath + ' doesn\'t currently exist in this repository.');
+export async function openFile(repo: string, filePath: string, hash: string | null = null, dataSource: DataSource | null = null, viewColumn: vscode.ViewColumn | null = null) {
+	let newFilePath = filePath;
+	let newAbsoluteFilePath = path.join(repo, newFilePath);
+	let fileExists = await doesFileExist(newAbsoluteFilePath);
+	if (!fileExists && hash !== null && dataSource !== null) {
+		const renamedFilePath = await dataSource.getNewPathOfRenamedFile(repo, hash, filePath);
+		if (renamedFilePath !== null) {
+			const renamedAbsoluteFilePath = path.join(repo, renamedFilePath);
+			if (await doesFileExist(renamedAbsoluteFilePath)) {
+				newFilePath = renamedFilePath;
+				newAbsoluteFilePath = renamedAbsoluteFilePath;
+				fileExists = true;
 			}
-		});
-	});
+		}
+	}
+
+	if (fileExists) {
+		return vscode.commands.executeCommand('vscode.open', vscode.Uri.file(newAbsoluteFilePath), {
+			preview: true,
+			viewColumn: viewColumn === null ? getConfig().openNewTabEditorGroup : viewColumn
+		}).then(
+			() => null,
+			() => 'Visual Studio Code was unable to open ' + newFilePath + '.'
+		);
+	} else {
+		return 'The file ' + newFilePath + ' doesn\'t currently exist in this repository.';
+	}
 }
 
 /**
@@ -394,15 +417,27 @@ export function viewDiff(repo: string, fromHash: string, toHash: string, oldFile
  * @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.
+ * @param dataSource A DataSource instance, that's used to check if the file has been renamed.
  * @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));
-		});
-	});
+export async function viewDiffWithWorkingFile(repo: string, hash: string, filePath: string, dataSource: DataSource) {
+	let newFilePath = filePath;
+	let fileExists = await doesFileExist(path.join(repo, newFilePath));
+	if (!fileExists) {
+		const renamedFilePath = await dataSource.getNewPathOfRenamedFile(repo, hash, filePath);
+		if (renamedFilePath !== null && await doesFileExist(path.join(repo, renamedFilePath))) {
+			newFilePath = renamedFilePath;
+			fileExists = true;
+		}
+	}
+
+	const type = fileExists
+		? filePath === newFilePath
+			? GitFileStatus.Modified
+			: GitFileStatus.Renamed
+		: GitFileStatus.Deleted;
+
+	return viewDiff(repo, hash, UNCOMMITTED, filePath, newFilePath, type);
 }
 
 /**
@@ -412,7 +447,7 @@ export function viewDiffWithWorkingFile(repo: string, hash: string, filePath: st
  * @param filePath The relative path of the file within the repository.
  * @returns A promise resolving to the ErrorInfo of the executed command.
  */
-export async function viewFileAtRevision(repo: string, hash: string, filePath: string) {
+export function viewFileAtRevision(repo: string, hash: string, filePath: string) {
 	const pathComponents = filePath.split('/');
 	const title = abbrevCommit(hash) + ': ' + pathComponents[pathComponents.length - 1];
 

tests/commands.test.ts

@@ -1210,7 +1210,7 @@ describe('CommandManager', () => {
 			await vscode.commands.executeCommand('git-graph.openFile', encodeDiffDocUri('/path/to/repo', 'subfolder/modified.txt', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', GitFileStatus.Modified, DiffSide.New));
 
 			// Assert
-			expect(spyOnOpenFile).toHaveBeenCalledWith('/path/to/repo', 'subfolder/modified.txt', vscode.ViewColumn.Active);
+			expect(spyOnOpenFile).toHaveBeenCalledWith('/path/to/repo', 'subfolder/modified.txt', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', dataSource, vscode.ViewColumn.Active);
 		});
 
 		it('Should open the file of the active text editor', async () => {
@@ -1221,7 +1221,7 @@ describe('CommandManager', () => {
 			await vscode.commands.executeCommand('git-graph.openFile');
 
 			// Assert
-			expect(spyOnOpenFile).toHaveBeenCalledWith('/path/to/repo', 'subfolder/modified.txt', vscode.ViewColumn.Active);
+			expect(spyOnOpenFile).toHaveBeenCalledWith('/path/to/repo', 'subfolder/modified.txt', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', dataSource, vscode.ViewColumn.Active);
 		});
 
 		it('Should display an error message when no URI is provided', async () => {
@@ -1257,7 +1257,7 @@ describe('CommandManager', () => {
 			await vscode.commands.executeCommand('git-graph.openFile');
 
 			// Assert
-			expect(spyOnOpenFile).toHaveBeenCalledWith('/path/to/repo', 'subfolder/modified.txt', vscode.ViewColumn.Active);
+			expect(spyOnOpenFile).toHaveBeenCalledWith('/path/to/repo', 'subfolder/modified.txt', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', dataSource, vscode.ViewColumn.Active);
 			expect(vscode.window.showErrorMessage).toHaveBeenCalledWith('Unable to Open File: Error Message');
 		});
 	});

tests/dataSource.test.ts

@@ -3796,6 +3796,44 @@ describe('DataSource', () => {
 		});
 	});
 
+	describe('getNewPathOfRenamedFile', () => {
+		it('Should return the new path of a file that was renamed', async () => {
+			// Setup
+			mockGitSuccessOnce(['R100', 'dir/renamed-old.txt', 'dir/renamed-new.txt', ''].join('\0'));
+
+			// Run
+			const result = await dataSource.getNewPathOfRenamedFile('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', 'dir/renamed-old.txt');
+
+			// Assert
+			expect(result).toBe('dir/renamed-new.txt');
+			expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--name-status', '--find-renames', '--diff-filter=R', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' }));
+		});
+
+		it('Should return NULL when a file wasn\'t renamed', async () => {
+			// Setup
+			mockGitSuccessOnce(['R100', 'dir/renamed-old.txt', 'dir/renamed-new.txt', ''].join('\0'));
+
+			// Run
+			const result = await dataSource.getNewPathOfRenamedFile('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', 'dir/deleted.txt');
+
+			// Assert
+			expect(result).toBe(null);
+			expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--name-status', '--find-renames', '--diff-filter=R', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' }));
+		});
+
+		it('Should return NULL when git threw an error', async () => {
+			// Setup
+			mockGitThrowingErrorOnce();
+
+			// Run
+			const result = await dataSource.getNewPathOfRenamedFile('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', 'dir/deleted.txt');
+
+			// Assert
+			expect(result).toBe(null);
+			expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--name-status', '--find-renames', '--diff-filter=R', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' }));
+		});
+	});
+
 	describe('getTagDetails', () => {
 		it('Should return the tags details', async () => {
 			// Setup