Skip to content

Commit fa30880

Browse files
committed
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
1 parent 89f2fe4 commit fa30880

File tree

7 files changed

+112
-71
lines changed

7 files changed

+112
-71
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
1818

1919
### Fixed
2020

21+
- Fixes [#3023](https://github.com/gitkraken/vscode-gitlens/issues/3023) - "Unable to show blame. Invalid or missing blame.ignoreRevsFile" with valid ignore revs file
2122
- Fixes [#3018](https://github.com/gitkraken/vscode-gitlens/issues/3018) - Line blame overlay is broken when commit message contains a `)`
2223
- Fixes [#2625](https://github.com/gitkraken/vscode-gitlens/issues/2625) - full issue ref has escape characters that break hover links
2324
- Fixes stuck busy state of the _Commit Details_ Explain AI panel after canceling a request

package.json

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4031,7 +4031,9 @@
40314031
"suppressRebaseSwitchToTextWarning": false,
40324032
"suppressIntegrationDisconnectedTooManyFailedRequestsWarning": false,
40334033
"suppressIntegrationRequestFailed500Warning": false,
4034-
"suppressIntegrationRequestTimedOutWarning": false
4034+
"suppressIntegrationRequestTimedOutWarning": false,
4035+
"suppressBlameInvalidIgnoreRevsFileWarning": false,
4036+
"suppressBlameInvalidIgnoreRevsFileBadRevisionWarning": false
40354037
},
40364038
"properties": {
40374039
"suppressCommitHasNoPreviousCommitWarning": {
@@ -4103,6 +4105,16 @@
41034105
"type": "boolean",
41044106
"default": false,
41054107
"description": "Integration Request Timed Out Warning"
4108+
},
4109+
"suppressBlameInvalidIgnoreRevsFileWarning": {
4110+
"type": "boolean",
4111+
"default": false,
4112+
"description": "Invalid Blame IgnoreRevs File Warning"
4113+
},
4114+
"suppressBlameInvalidIgnoreRevsFileBadRevisionWarning": {
4115+
"type": "boolean",
4116+
"default": false,
4117+
"description": "Invalid Revision in Blame IgnoreRevs File Warning"
41064118
}
41074119
},
41084120
"additionalProperties": false,

src/config.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,9 @@ export type SuppressedMessages =
537537
| 'suppressRebaseSwitchToTextWarning'
538538
| 'suppressIntegrationDisconnectedTooManyFailedRequestsWarning'
539539
| 'suppressIntegrationRequestFailed500Warning'
540-
| 'suppressIntegrationRequestTimedOutWarning';
540+
| 'suppressIntegrationRequestTimedOutWarning'
541+
| 'suppressBlameInvalidIgnoreRevsFileWarning'
542+
| 'suppressBlameInvalidIgnoreRevsFileBadRevisionWarning';
541543

542544
export interface ViewsCommonConfig {
543545
readonly defaultItemLimit: number;

src/env/node/git/git.ts

Lines changed: 42 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { GlyphChars } from '../../../constants';
1010
import type { GitCommandOptions, GitSpawnOptions } from '../../../git/commandOptions';
1111
import { GitErrorHandling } from '../../../git/commandOptions';
1212
import {
13+
BlameIgnoreRevsFileBadRevisionError,
1314
BlameIgnoreRevsFileError,
1415
CherryPickError,
1516
CherryPickErrorReason,
@@ -35,7 +36,7 @@ import { parseGitTagsDefaultFormat } from '../../../git/parsers/tagParser';
3536
import { splitAt } from '../../../system/array';
3637
import { configuration } from '../../../system/configuration';
3738
import { log } from '../../../system/decorators/log';
38-
import { join } from '../../../system/iterable';
39+
import { count, join } from '../../../system/iterable';
3940
import { Logger } from '../../../system/logger';
4041
import { slowCallWarningThreshold } from '../../../system/logger.constants';
4142
import { getLogScope } from '../../../system/logger.scope';
@@ -70,12 +71,13 @@ const textDecoder = new TextDecoder('utf8');
7071
const rootSha = '4b825dc642cb6eb9a060e54bf8d69288fbee4904';
7172

7273
export const GitErrors = {
73-
badIgnoreRevsFile: /could not open object name list: (.*)\s/i,
7474
badRevision: /bad revision '(.*?)'/i,
7575
cantLockRef: /cannot lock ref|unable to update local ref/i,
7676
changesWouldBeOverwritten: /Your local changes to the following files would be overwritten/i,
7777
commitChangesFirst: /Please, commit your changes before you can/i,
7878
conflict: /^CONFLICT \([^)]+\): \b/m,
79+
invalidObjectName: /invalid object name: (.*)\s/i,
80+
invalidObjectNameList: /could not open object name list: (.*)\s/i,
7981
noFastForward: /\(non-fast-forward\)/i,
8082
noMergeBase: /no merge base/i,
8183
noRemoteRepositorySpecified: /No remote repository specified\./i,
@@ -386,20 +388,37 @@ export class Git {
386388
async blame(
387389
repoPath: string | undefined,
388390
fileName: string,
389-
ref?: string,
390-
options: { args?: string[] | null; ignoreWhitespace?: boolean; startLine?: number; endLine?: number } = {},
391+
options?: ({ ref: string | undefined; contents?: never } | { contents: string; ref?: never }) & {
392+
args?: string[] | null;
393+
correlationKey?: string;
394+
ignoreWhitespace?: boolean;
395+
startLine?: number;
396+
endLine?: number;
397+
},
391398
) {
392399
const [file, root] = splitPath(fileName, repoPath, true);
393400

394401
const params = ['blame', '--root', '--incremental'];
395402

396-
if (options.ignoreWhitespace) {
403+
if (options?.ignoreWhitespace) {
397404
params.push('-w');
398405
}
399-
if (options.startLine != null && options.endLine != null) {
406+
if (options?.startLine != null && options.endLine != null) {
400407
params.push(`-L ${options.startLine},${options.endLine}`);
401408
}
402-
if (options.args != null) {
409+
if (options?.args != null) {
410+
// See if the args contains a value like: `--ignore-revs-file <file>` or `--ignore-revs-file=<file>` to account for user error
411+
// If so split it up into two args
412+
const argIndex = options.args.findIndex(
413+
arg => arg !== '--ignore-revs-file' && arg.startsWith('--ignore-revs-file'),
414+
);
415+
if (argIndex !== -1) {
416+
const match = /^--ignore-revs-file\s*=?\s*(.*)$/.exec(options.args[argIndex]);
417+
if (match != null) {
418+
options.args.splice(argIndex, 1, '--ignore-revs-file', match[1]);
419+
}
420+
}
421+
403422
params.push(...options.args);
404423
}
405424

@@ -444,74 +463,43 @@ export class Git {
444463
}
445464

446465
let stdin;
447-
if (ref) {
448-
if (isUncommittedStaged(ref)) {
466+
if (options?.contents != null) {
467+
// Pipe the blame contents to stdin
468+
params.push('--contents', '-');
469+
470+
stdin = options.contents;
471+
} else if (options?.ref) {
472+
if (isUncommittedStaged(options.ref)) {
449473
// Pipe the blame contents to stdin
450474
params.push('--contents', '-');
451475

452476
// Get the file contents for the staged version using `:`
453477
stdin = await this.show<string>(repoPath, fileName, ':');
454478
} else {
455-
params.push(ref);
479+
params.push(options.ref);
456480
}
457481
}
458482

459-
try {
460-
const blame = await this.git<string>({ cwd: root, stdin: stdin }, ...params, '--', file);
461-
return blame;
462-
} catch (ex) {
463-
// 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
464-
const match = GitErrors.badIgnoreRevsFile.exec(ex.message);
465-
if (match != null) {
466-
throw new BlameIgnoreRevsFileError(match[1], ex);
467-
}
468-
throw ex;
469-
}
470-
}
471-
472-
async blame__contents(
473-
repoPath: string | undefined,
474-
fileName: string,
475-
contents: string,
476-
options: {
477-
args?: string[] | null;
478-
correlationKey?: string;
479-
ignoreWhitespace?: boolean;
480-
startLine?: number;
481-
endLine?: number;
482-
} = {},
483-
) {
484-
const [file, root] = splitPath(fileName, repoPath, true);
485-
486-
const params = ['blame', '--root', '--incremental'];
487-
488-
if (options.ignoreWhitespace) {
489-
params.push('-w');
490-
}
491-
if (options.startLine != null && options.endLine != null) {
492-
params.push(`-L ${options.startLine},${options.endLine}`);
493-
}
494-
if (options.args != null) {
495-
params.push(...options.args);
496-
}
497-
498-
// Pipe the blame contents to stdin
499-
params.push('--contents', '-');
500-
501483
try {
502484
const blame = await this.git<string>(
503-
{ cwd: root, stdin: contents, correlationKey: options.correlationKey },
485+
{ cwd: root, stdin: stdin, correlationKey: options?.correlationKey },
504486
...params,
505487
'--',
506488
file,
507489
);
508490
return blame;
509491
} catch (ex) {
510492
// 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
511-
const match = GitErrors.badIgnoreRevsFile.exec(ex.message);
493+
let match = GitErrors.invalidObjectNameList.exec(ex.message);
512494
if (match != null) {
513495
throw new BlameIgnoreRevsFileError(match[1], ex);
514496
}
497+
498+
match = GitErrors.invalidObjectName.exec(ex.message);
499+
if (match != null) {
500+
throw new BlameIgnoreRevsFileBadRevisionError(match[1], ex);
501+
}
502+
515503
throw ex;
516504
}
517505
}

src/env/node/git/localGitProvider.ts

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { emojify } from '../../../emojis';
1717
import { Features } from '../../../features';
1818
import { GitErrorHandling } from '../../../git/commandOptions';
1919
import {
20+
BlameIgnoreRevsFileBadRevisionError,
2021
BlameIgnoreRevsFileError,
2122
CherryPickError,
2223
CherryPickErrorReason,
@@ -150,6 +151,7 @@ import { getRemoteProviderMatcher, loadRemoteProviders } from '../../../git/remo
150151
import type { GitSearch, GitSearchResultData, GitSearchResults, SearchQuery } from '../../../git/search';
151152
import { getGitArgsFromSearchQuery, getSearchQueryComparisonKey } from '../../../git/search';
152153
import {
154+
showBlameInvalidIgnoreRevsFileWarningMessage,
153155
showGenericErrorMessage,
154156
showGitDisabledErrorMessage,
155157
showGitInvalidConfigErrorMessage,
@@ -1748,7 +1750,8 @@ export class LocalGitProvider implements GitProvider, Disposable {
17481750
const [relativePath, root] = paths;
17491751

17501752
try {
1751-
const data = await this.git.blame(root, relativePath, uri.sha, {
1753+
const data = await this.git.blame(root, relativePath, {
1754+
ref: uri.sha,
17521755
args: configuration.get('advanced.blame.customArguments'),
17531756
ignoreWhitespace: configuration.get('blame.ignoreWhitespace'),
17541757
});
@@ -1769,8 +1772,8 @@ export class LocalGitProvider implements GitProvider, Disposable {
17691772
document.state.setBlame(key, value);
17701773
document.setBlameFailure(ex);
17711774

1772-
if (ex instanceof BlameIgnoreRevsFileError) {
1773-
void window.showErrorMessage(ex.friendlyMessage);
1775+
if (ex instanceof BlameIgnoreRevsFileError || ex instanceof BlameIgnoreRevsFileBadRevisionError) {
1776+
void showBlameInvalidIgnoreRevsFileWarningMessage(ex);
17741777
}
17751778

17761779
return emptyPromise as Promise<GitBlame>;
@@ -1831,7 +1834,8 @@ export class LocalGitProvider implements GitProvider, Disposable {
18311834
const [relativePath, root] = paths;
18321835

18331836
try {
1834-
const data = await this.git.blame__contents(root, relativePath, contents, {
1837+
const data = await this.git.blame(root, relativePath, {
1838+
contents: contents,
18351839
args: configuration.get('advanced.blame.customArguments'),
18361840
correlationKey: `:${key}`,
18371841
ignoreWhitespace: configuration.get('blame.ignoreWhitespace'),
@@ -1853,8 +1857,8 @@ export class LocalGitProvider implements GitProvider, Disposable {
18531857
document.state.setBlame(key, value);
18541858
document.setBlameFailure(ex);
18551859

1856-
if (ex instanceof BlameIgnoreRevsFileError) {
1857-
void window.showErrorMessage(ex.friendlyMessage);
1860+
if (ex instanceof BlameIgnoreRevsFileError || ex instanceof BlameIgnoreRevsFileBadRevisionError) {
1861+
void showBlameInvalidIgnoreRevsFileWarningMessage(ex);
18581862
}
18591863

18601864
return emptyPromise as Promise<GitBlame>;
@@ -1903,7 +1907,8 @@ export class LocalGitProvider implements GitProvider, Disposable {
19031907
const [relativePath, root] = splitPath(uri, uri.repoPath);
19041908

19051909
try {
1906-
const data = await this.git.blame(root, relativePath, uri.sha, {
1910+
const data = await this.git.blame(root, relativePath, {
1911+
ref: uri.sha,
19071912
args: configuration.get('advanced.blame.customArguments'),
19081913
ignoreWhitespace: configuration.get('blame.ignoreWhitespace'),
19091914
startLine: lineToBlame,
@@ -1919,8 +1924,8 @@ export class LocalGitProvider implements GitProvider, Disposable {
19191924
};
19201925
} catch (ex) {
19211926
Logger.error(ex, scope);
1922-
if (ex instanceof BlameIgnoreRevsFileError) {
1923-
void window.showErrorMessage(ex.friendlyMessage);
1927+
if (ex instanceof BlameIgnoreRevsFileError || ex instanceof BlameIgnoreRevsFileBadRevisionError) {
1928+
void showBlameInvalidIgnoreRevsFileWarningMessage(ex);
19241929
}
19251930

19261931
return undefined;
@@ -1959,7 +1964,8 @@ export class LocalGitProvider implements GitProvider, Disposable {
19591964
const [relativePath, root] = splitPath(uri, uri.repoPath);
19601965

19611966
try {
1962-
const data = await this.git.blame__contents(root, relativePath, contents, {
1967+
const data = await this.git.blame(root, relativePath, {
1968+
contents: contents,
19631969
args: configuration.get('advanced.blame.customArguments'),
19641970
ignoreWhitespace: configuration.get('blame.ignoreWhitespace'),
19651971
startLine: lineToBlame,

src/git/errors.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,31 @@ export class BlameIgnoreRevsFileError extends Error {
1111
return ex instanceof BlameIgnoreRevsFileError;
1212
}
1313

14-
readonly friendlyMessage: string;
15-
1614
constructor(
17-
filename: string,
15+
public readonly fileName: string,
1816
public readonly original?: Error,
1917
) {
20-
super(`Invalid blame.ignoreRevsFile: '${filename}'`);
18+
super(`Invalid blame.ignoreRevsFile: '${fileName}'`);
2119

22-
this.friendlyMessage = `Unable to show blame. Invalid or missing blame.ignoreRevsFile (${filename}) specified in your Git config.`;
2320
Error.captureStackTrace?.(this, BlameIgnoreRevsFileError);
2421
}
2522
}
2623

24+
export class BlameIgnoreRevsFileBadRevisionError extends Error {
25+
static is(ex: unknown): ex is BlameIgnoreRevsFileBadRevisionError {
26+
return ex instanceof BlameIgnoreRevsFileBadRevisionError;
27+
}
28+
29+
constructor(
30+
public readonly revision: string,
31+
public readonly original?: Error,
32+
) {
33+
super(`Invalid revision in blame.ignoreRevsFile: '${revision}'`);
34+
35+
Error.captureStackTrace?.(this, BlameIgnoreRevsFileBadRevisionError);
36+
}
37+
}
38+
2739
export const enum StashApplyErrorReason {
2840
WorkingChanges = 1,
2941
}

src/messages.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,31 @@ import type { MessageItem } from 'vscode';
22
import { ConfigurationTarget, window } from 'vscode';
33
import type { SuppressedMessages } from './config';
44
import { Commands } from './constants';
5+
import type { BlameIgnoreRevsFileError } from './git/errors';
6+
import { BlameIgnoreRevsFileBadRevisionError } from './git/errors';
57
import type { GitCommit } from './git/models/commit';
68
import { executeCommand } from './system/command';
79
import { configuration } from './system/configuration';
810
import { Logger } from './system/logger';
911

12+
export function showBlameInvalidIgnoreRevsFileWarningMessage(
13+
ex: BlameIgnoreRevsFileError | BlameIgnoreRevsFileBadRevisionError,
14+
): Promise<MessageItem | undefined> {
15+
if (ex instanceof BlameIgnoreRevsFileBadRevisionError) {
16+
return showMessage(
17+
'error',
18+
`Unable to show blame. Invalid revision (${ex.revision}) specified in the blame.ignoreRevsFile in your Git config.`,
19+
'suppressBlameInvalidIgnoreRevsFileBadRevisionWarning',
20+
);
21+
}
22+
23+
return showMessage(
24+
'error',
25+
`Unable to show blame. Invalid or missing blame.ignoreRevsFile (${ex.fileName}) specified in your Git config.`,
26+
'suppressBlameInvalidIgnoreRevsFileWarning',
27+
);
28+
}
29+
1030
export function showCommitHasNoPreviousCommitWarningMessage(commit?: GitCommit): Promise<MessageItem | undefined> {
1131
if (commit == null) {
1232
return showMessage('info', 'There is no previous commit.', 'suppressCommitHasNoPreviousCommitWarning');

0 commit comments

Comments
 (0)