Fixes #3023 unifies file & dirty blame

Adds don't prompt again to error prompts
Adds specific error handling for invalid revision vs invalid file
Adds better fallback handling of user entered custom args

CHANGELOG.md

@@ -18,6 +18,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
 
 ### Fixed
 
+- Fixes [#3023](https://github.com/gitkraken/vscode-gitlens/issues/3023) - "Unable to show blame. Invalid or missing blame.ignoreRevsFile" with valid ignore revs file
 - Fixes [#3018](https://github.com/gitkraken/vscode-gitlens/issues/3018) - Line blame overlay is broken when commit message contains a `)`
 - Fixes [#2625](https://github.com/gitkraken/vscode-gitlens/issues/2625) - full issue ref has escape characters that break hover links
 - Fixes stuck busy state of the _Commit Details_ Explain AI panel after canceling a request

package.json

@@ -4031,7 +4031,9 @@
 							"suppressRebaseSwitchToTextWarning": false,
 							"suppressIntegrationDisconnectedTooManyFailedRequestsWarning": false,
 							"suppressIntegrationRequestFailed500Warning": false,
-							"suppressIntegrationRequestTimedOutWarning": false
+							"suppressIntegrationRequestTimedOutWarning": false,
+							"suppressBlameInvalidIgnoreRevsFileWarning": false,
+							"suppressBlameInvalidIgnoreRevsFileBadRevisionWarning": false
 						},
 						"properties": {
 							"suppressCommitHasNoPreviousCommitWarning": {
@@ -4103,6 +4105,16 @@
 								"type": "boolean",
 								"default": false,
 								"description": "Integration Request Timed Out Warning"
+							},
+							"suppressBlameInvalidIgnoreRevsFileWarning": {
+								"type": "boolean",
+								"default": false,
+								"description": "Invalid Blame IgnoreRevs File Warning"
+							},
+							"suppressBlameInvalidIgnoreRevsFileBadRevisionWarning": {
+								"type": "boolean",
+								"default": false,
+								"description": "Invalid Revision in Blame IgnoreRevs File Warning"
 							}
 						},
 						"additionalProperties": false,

src/config.ts

@@ -537,7 +537,9 @@ export type SuppressedMessages =
 	| 'suppressRebaseSwitchToTextWarning'
 	| 'suppressIntegrationDisconnectedTooManyFailedRequestsWarning'
 	| 'suppressIntegrationRequestFailed500Warning'
-	| 'suppressIntegrationRequestTimedOutWarning';
+	| 'suppressIntegrationRequestTimedOutWarning'
+	| 'suppressBlameInvalidIgnoreRevsFileWarning'
+	| 'suppressBlameInvalidIgnoreRevsFileBadRevisionWarning';
 
 export interface ViewsCommonConfig {
 	readonly defaultItemLimit: number;

src/env/node/git/git.ts

@@ -10,6 +10,7 @@ import { GlyphChars } from '../../../constants';
 import type { GitCommandOptions, GitSpawnOptions } from '../../../git/commandOptions';
 import { GitErrorHandling } from '../../../git/commandOptions';
 import {
+	BlameIgnoreRevsFileBadRevisionError,
 	BlameIgnoreRevsFileError,
 	CherryPickError,
 	CherryPickErrorReason,
@@ -35,7 +36,7 @@ import { parseGitTagsDefaultFormat } from '../../../git/parsers/tagParser';
 import { splitAt } from '../../../system/array';
 import { configuration } from '../../../system/configuration';
 import { log } from '../../../system/decorators/log';
-import { join } from '../../../system/iterable';
+import { count, join } from '../../../system/iterable';
 import { Logger } from '../../../system/logger';
 import { slowCallWarningThreshold } from '../../../system/logger.constants';
 import { getLogScope } from '../../../system/logger.scope';
@@ -70,12 +71,13 @@ const textDecoder = new TextDecoder('utf8');
 const rootSha = '4b825dc642cb6eb9a060e54bf8d69288fbee4904';
 
 export const GitErrors = {
-	badIgnoreRevsFile: /could not open object name list: (.*)\s/i,
 	badRevision: /bad revision '(.*?)'/i,
 	cantLockRef: /cannot lock ref|unable to update local ref/i,
 	changesWouldBeOverwritten: /Your local changes to the following files would be overwritten/i,
 	commitChangesFirst: /Please, commit your changes before you can/i,
 	conflict: /^CONFLICT [)]+: \b/m,
+	invalidObjectName: /invalid object name: (.*)\s/i,
+	invalidObjectNameList: /could not open object name list: (.*)\s/i,
 	noFastForward: /non−fast−forward/i,
 	noMergeBase: /no merge base/i,
 	noRemoteRepositorySpecified: /No remote repository specified\./i,
@@ -386,20 +388,37 @@ export class Git {
 	async blame(
 		repoPath: string | undefined,
 		fileName: string,
-		ref?: string,
-		options: { args?: string[] | null; ignoreWhitespace?: boolean; startLine?: number; endLine?: number } = {},
+		options?: ({ ref: string | undefined; contents?: never } | { contents: string; ref?: never }) & {
+			args?: string[] | null;
+			correlationKey?: string;
+			ignoreWhitespace?: boolean;
+			startLine?: number;
+			endLine?: number;
+		},
 	) {
 		const [file, root] = splitPath(fileName, repoPath, true);
 
 		const params = ['blame', '--root', '--incremental'];
 
-		if (options.ignoreWhitespace) {
+		if (options?.ignoreWhitespace) {
 			params.push('-w');
 		}
-		if (options.startLine != null && options.endLine != null) {
+		if (options?.startLine != null && options.endLine != null) {
 			params.push(`-L ${options.startLine},${options.endLine}`);
 		}
-		if (options.args != null) {
+		if (options?.args != null) {
+			// See if the args contains a value like: `--ignore-revs-file <file>` or `--ignore-revs-file=<file>` to account for user error
+			// If so split it up into two args
+			const argIndex = options.args.findIndex(
+				arg => arg !== '--ignore-revs-file' && arg.startsWith('--ignore-revs-file'),
+			);
+			if (argIndex !== -1) {
+				const match = /^--ignore-revs-file\s*=?\s*(.*)$/.exec(options.args[argIndex]);
+				if (match != null) {
+					options.args.splice(argIndex, 1, '--ignore-revs-file', match[1]);
+				}
+			}
+
 			params.push(...options.args);
 		}
 
@@ -444,63 +463,26 @@ export class Git {
 		}
 
 		let stdin;
-		if (ref) {
-			if (isUncommittedStaged(ref)) {
+		if (options?.contents != null) {
+			// Pipe the blame contents to stdin
+			params.push('--contents', '-');
+
+			stdin = options.contents;
+		} else if (options?.ref) {
+			if (isUncommittedStaged(options.ref)) {
 				// Pipe the blame contents to stdin
 				params.push('--contents', '-');
 
 				// Get the file contents for the staged version using `:`
 				stdin = await this.show<string>(repoPath, fileName, ':');
 			} else {
-				params.push(ref);
+				params.push(options.ref);
 			}
 		}
 
 		try {
-			const blame = await this.git<string>({ cwd: root, stdin: stdin }, ...params, '--', file);
-			return blame;
-		} catch (ex) {
-			// Since `-c blame.ignoreRevsFile=` doesn't seem to work (unlike as the docs suggest), try to detect the error and throw a more helpful one
-			const match = GitErrors.badIgnoreRevsFile.exec(ex.message);
-			if (match != null) {
-				throw new BlameIgnoreRevsFileError(match[1], ex);
-			}
-			throw ex;
-		}
-	}
-
-	async blame__contents(
-		repoPath: string | undefined,
-		fileName: string,
-		contents: string,
-		options: {
-			args?: string[] | null;
-			correlationKey?: string;
-			ignoreWhitespace?: boolean;
-			startLine?: number;
-			endLine?: number;
-		} = {},
-	) {
-		const [file, root] = splitPath(fileName, repoPath, true);
-
-		const params = ['blame', '--root', '--incremental'];
-
-		if (options.ignoreWhitespace) {
-			params.push('-w');
-		}
-		if (options.startLine != null && options.endLine != null) {
-			params.push(`-L ${options.startLine},${options.endLine}`);
-		}
-		if (options.args != null) {
-			params.push(...options.args);
-		}
-
-		// Pipe the blame contents to stdin
-		params.push('--contents', '-');
-
-		try {
 			const blame = await this.git<string>(
-				{ cwd: root, stdin: contents, correlationKey: options.correlationKey },
+				{ cwd: root, stdin: stdin, correlationKey: options?.correlationKey },
 				...params,
 				'--',
 				file,
@@ -508,10 +490,16 @@ export class Git {
 			return blame;
 		} catch (ex) {
 			// Since `-c blame.ignoreRevsFile=` doesn't seem to work (unlike as the docs suggest), try to detect the error and throw a more helpful one
-			const match = GitErrors.badIgnoreRevsFile.exec(ex.message);
+			let match = GitErrors.invalidObjectNameList.exec(ex.message);
 			if (match != null) {
 				throw new BlameIgnoreRevsFileError(match[1], ex);
 			}
+
+			match = GitErrors.invalidObjectName.exec(ex.message);
+			if (match != null) {
+				throw new BlameIgnoreRevsFileBadRevisionError(match[1], ex);
+			}
+
 			throw ex;
 		}
 	}

src/env/node/git/localGitProvider.ts

@@ -17,6 +17,7 @@ import { emojify } from '../../../emojis';
 import { Features } from '../../../features';
 import { GitErrorHandling } from '../../../git/commandOptions';
 import {
+	BlameIgnoreRevsFileBadRevisionError,
 	BlameIgnoreRevsFileError,
 	CherryPickError,
 	CherryPickErrorReason,
@@ -150,6 +151,7 @@ import { getRemoteProviderMatcher, loadRemoteProviders } from '../../../git/remo
 import type { GitSearch, GitSearchResultData, GitSearchResults, SearchQuery } from '../../../git/search';
 import { getGitArgsFromSearchQuery, getSearchQueryComparisonKey } from '../../../git/search';
 import {
+	showBlameInvalidIgnoreRevsFileWarningMessage,
 	showGenericErrorMessage,
 	showGitDisabledErrorMessage,
 	showGitInvalidConfigErrorMessage,
@@ -1748,7 +1750,8 @@ export class LocalGitProvider implements GitProvider, Disposable {
 		const [relativePath, root] = paths;
 
 		try {
-			const data = await this.git.blame(root, relativePath, uri.sha, {
+			const data = await this.git.blame(root, relativePath, {
+				ref: uri.sha,
 				args: configuration.get('advanced.blame.customArguments'),
 				ignoreWhitespace: configuration.get('blame.ignoreWhitespace'),
 			});
@@ -1769,8 +1772,8 @@ export class LocalGitProvider implements GitProvider, Disposable {
 				document.state.setBlame(key, value);
 				document.setBlameFailure(ex);
 
-				if (ex instanceof BlameIgnoreRevsFileError) {
-					void window.showErrorMessage(ex.friendlyMessage);
+				if (ex instanceof BlameIgnoreRevsFileError || ex instanceof BlameIgnoreRevsFileBadRevisionError) {
+					void showBlameInvalidIgnoreRevsFileWarningMessage(ex);
 				}
 
 				return emptyPromise as Promise<GitBlame>;
@@ -1831,7 +1834,8 @@ export class LocalGitProvider implements GitProvider, Disposable {
 		const [relativePath, root] = paths;
 
 		try {
-			const data = await this.git.blame__contents(root, relativePath, contents, {
+			const data = await this.git.blame(root, relativePath, {
+				contents: contents,
 				args: configuration.get('advanced.blame.customArguments'),
 				correlationKey: `:${key}`,
 				ignoreWhitespace: configuration.get('blame.ignoreWhitespace'),
@@ -1853,8 +1857,8 @@ export class LocalGitProvider implements GitProvider, Disposable {
 				document.state.setBlame(key, value);
 				document.setBlameFailure(ex);
 
-				if (ex instanceof BlameIgnoreRevsFileError) {
-					void window.showErrorMessage(ex.friendlyMessage);
+				if (ex instanceof BlameIgnoreRevsFileError || ex instanceof BlameIgnoreRevsFileBadRevisionError) {
+					void showBlameInvalidIgnoreRevsFileWarningMessage(ex);
 				}
 
 				return emptyPromise as Promise<GitBlame>;
@@ -1903,7 +1907,8 @@ export class LocalGitProvider implements GitProvider, Disposable {
 		const [relativePath, root] = splitPath(uri, uri.repoPath);
 
 		try {
-			const data = await this.git.blame(root, relativePath, uri.sha, {
+			const data = await this.git.blame(root, relativePath, {
+				ref: uri.sha,
 				args: configuration.get('advanced.blame.customArguments'),
 				ignoreWhitespace: configuration.get('blame.ignoreWhitespace'),
 				startLine: lineToBlame,
@@ -1919,8 +1924,8 @@ export class LocalGitProvider implements GitProvider, Disposable {
 			};
 		} catch (ex) {
 			Logger.error(ex, scope);
-			if (ex instanceof BlameIgnoreRevsFileError) {
-				void window.showErrorMessage(ex.friendlyMessage);
+			if (ex instanceof BlameIgnoreRevsFileError || ex instanceof BlameIgnoreRevsFileBadRevisionError) {
+				void showBlameInvalidIgnoreRevsFileWarningMessage(ex);
 			}
 
 			return undefined;
@@ -1959,7 +1964,8 @@ export class LocalGitProvider implements GitProvider, Disposable {
 		const [relativePath, root] = splitPath(uri, uri.repoPath);
 
 		try {
-			const data = await this.git.blame__contents(root, relativePath, contents, {
+			const data = await this.git.blame(root, relativePath, {
+				contents: contents,
 				args: configuration.get('advanced.blame.customArguments'),
 				ignoreWhitespace: configuration.get('blame.ignoreWhitespace'),
 				startLine: lineToBlame,

src/git/errors.ts

@@ -11,19 +11,31 @@ export class BlameIgnoreRevsFileError extends Error {
 		return ex instanceof BlameIgnoreRevsFileError;
 	}
 
-	readonly friendlyMessage: string;
-
 	constructor(
-		filename: string,
+		public readonly fileName: string,
 		public readonly original?: Error,
 	) {
-		super(`Invalid blame.ignoreRevsFile: '${filename}'`);
+		super(`Invalid blame.ignoreRevsFile: '${fileName}'`);
 
-		this.friendlyMessage = `Unable to show blame. Invalid or missing blame.ignoreRevsFile (${filename}) specified in your Git config.`;
 		Error.captureStackTrace?.(this, BlameIgnoreRevsFileError);
 	}
 }
 
+export class BlameIgnoreRevsFileBadRevisionError extends Error {
+	static is(ex: unknown): ex is BlameIgnoreRevsFileBadRevisionError {
+		return ex instanceof BlameIgnoreRevsFileBadRevisionError;
+	}
+
+	constructor(
+		public readonly revision: string,
+		public readonly original?: Error,
+	) {
+		super(`Invalid revision in blame.ignoreRevsFile: '${revision}'`);
+
+		Error.captureStackTrace?.(this, BlameIgnoreRevsFileBadRevisionError);
+	}
+}
+
 export const enum StashApplyErrorReason {
 	WorkingChanges = 1,
 }

src/messages.ts

@@ -2,11 +2,31 @@ import type { MessageItem } from 'vscode';
 import { ConfigurationTarget, window } from 'vscode';
 import type { SuppressedMessages } from './config';
 import { Commands } from './constants';
+import type { BlameIgnoreRevsFileError } from './git/errors';
+import { BlameIgnoreRevsFileBadRevisionError } from './git/errors';
 import type { GitCommit } from './git/models/commit';
 import { executeCommand } from './system/command';
 import { configuration } from './system/configuration';
 import { Logger } from './system/logger';
 
+export function showBlameInvalidIgnoreRevsFileWarningMessage(
+	ex: BlameIgnoreRevsFileError | BlameIgnoreRevsFileBadRevisionError,
+): Promise<MessageItem | undefined> {
+	if (ex instanceof BlameIgnoreRevsFileBadRevisionError) {
+		return showMessage(
+			'error',
+			`Unable to show blame. Invalid revision (${ex.revision}) specified in the blame.ignoreRevsFile in your Git config.`,
+			'suppressBlameInvalidIgnoreRevsFileBadRevisionWarning',
+		);
+	}
+
+	return showMessage(
+		'error',
+		`Unable to show blame. Invalid or missing blame.ignoreRevsFile (${ex.fileName}) specified in your Git config.`,
+		'suppressBlameInvalidIgnoreRevsFileWarning',
+	);
+}
+
 export function showCommitHasNoPreviousCommitWarningMessage(commit?: GitCommit): Promise<MessageItem | undefined> {
 	if (commit == null) {
 		return showMessage('info', 'There is no previous commit.', 'suppressCommitHasNoPreviousCommitWarning');