From 92e37bdba6db179df92aecde8df773ae9bea7112 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Tue, 17 Oct 2023 23:55:29 -0400 Subject: [PATCH] Fixes #2952 error msg for missing ignore revs file --- CHANGELOG.md | 4 ++++ src/env/node/git/git.ts | 38 ++++++++++++++++++++++++++++-------- src/env/node/git/localGitProvider.ts | 29 ++++++++++++++++++++++----- src/git/errors.ts | 32 +++++++++++++++++++++++------- 4 files changed, 83 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 269d757..44b36459 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p ## [Unreleased] +### Fixed + +- Fixes [#2952](https://github.com/gitkraken/vscode-gitlens/issues/2952) - Inline blame not working because of missing ignoreRevsFile + ## [14.4.0] - 2023-10-13 ### Added diff --git a/src/env/node/git/git.ts b/src/env/node/git/git.ts index 8d93a4e..52169a0 100644 --- a/src/env/node/git/git.ts +++ b/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 { + BlameIgnoreRevsFileError, FetchError, FetchErrorReason, PullError, @@ -68,6 +69,7 @@ const textDecoder = new TextDecoder('utf8'); const rootSha = '4b825dc642cb6eb9a060e54bf8d69288fbee4904'; export const GitErrors = { + badIgnoreRevsFile: /could not open object name list: (.*)\w/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, @@ -464,10 +466,20 @@ export class Git { } } - return this.git({ cwd: root, stdin: stdin }, ...params, '--', file); + try { + const blame = await this.git({ 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; + } } - blame__contents( + async blame__contents( repoPath: string | undefined, fileName: string, contents: string, @@ -496,12 +508,22 @@ export class Git { // Pipe the blame contents to stdin params.push('--contents', '-'); - return this.git( - { cwd: root, stdin: contents, correlationKey: options.correlationKey }, - ...params, - '--', - file, - ); + try { + const blame = await this.git( + { cwd: root, stdin: contents, correlationKey: options.correlationKey }, + ...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; + } } branchOrTag__containsOrPointsAt( diff --git a/src/env/node/git/localGitProvider.ts b/src/env/node/git/localGitProvider.ts index af4da17..33b86b3 100644 --- a/src/env/node/git/localGitProvider.ts +++ b/src/env/node/git/localGitProvider.ts @@ -21,6 +21,7 @@ import { emojify } from '../../../emojis'; import { Features } from '../../../features'; import { GitErrorHandling } from '../../../git/commandOptions'; import { + BlameIgnoreRevsFileError, FetchError, GitSearchError, PullError, @@ -1463,19 +1464,24 @@ export class LocalGitProvider implements GitProvider, Disposable { const blame = parseGitBlame(this.container, data, root, await this.getCurrentUser(root)); return blame; } catch (ex) { + Logger.error(ex, scope); + // Trap and cache expected blame errors if (document.state != null) { const msg = ex?.toString() ?? ''; - Logger.debug(scope, `Cache replace (with empty promise): '${key}'`); + Logger.debug(scope, `Cache replace (with empty promise): '${key}'; reason=${msg}`); const value: CachedBlame = { item: emptyPromise as Promise, errorMessage: msg, }; document.state.setBlame(key, value); - document.setBlameFailure(); + if (ex instanceof BlameIgnoreRevsFileError) { + void window.showErrorMessage(ex.friendlyMessage); + } + return emptyPromise as Promise; } @@ -1544,18 +1550,24 @@ export class LocalGitProvider implements GitProvider, Disposable { const blame = parseGitBlame(this.container, data, root, await this.getCurrentUser(root)); return blame; } catch (ex) { + Logger.error(ex, scope); + // Trap and cache expected blame errors if (document.state != null) { const msg = ex?.toString() ?? ''; - Logger.debug(scope, `Cache replace (with empty promise): '${key}'`); + Logger.debug(scope, `Cache replace (with empty promise): '${key}'; reason=${msg}`); const value: CachedBlame = { item: emptyPromise as Promise, errorMessage: msg, }; document.state.setBlame(key, value); - document.setBlameFailure(); + + if (ex instanceof BlameIgnoreRevsFileError) { + void window.showErrorMessage(ex.friendlyMessage); + } + return emptyPromise as Promise; } @@ -1575,6 +1587,8 @@ export class LocalGitProvider implements GitProvider, Disposable { ): Promise { if (document?.isDirty) return this.getBlameForLineContents(uri, editorLine, document.getText(), options); + const scope = getLogScope(); + if (!options?.forceSingleLine && this.useCaching) { const blame = await this.getBlame(uri, document); if (blame == null) return undefined; @@ -1614,7 +1628,12 @@ export class LocalGitProvider implements GitProvider, Disposable { commit: first(blame.commits.values())!, line: blame.lines[editorLine], }; - } catch { + } catch (ex) { + Logger.error(ex, scope); + if (ex instanceof BlameIgnoreRevsFileError) { + void window.showErrorMessage(ex.friendlyMessage); + } + return undefined; } } diff --git a/src/git/errors.ts b/src/git/errors.ts index 8b4d5cb..de9639c 100644 --- a/src/git/errors.ts +++ b/src/git/errors.ts @@ -6,12 +6,30 @@ export class GitSearchError extends Error { } } +export class BlameIgnoreRevsFileError extends Error { + static is(ex: unknown): ex is BlameIgnoreRevsFileError { + return ex instanceof BlameIgnoreRevsFileError; + } + + readonly friendlyMessage: string; + + constructor( + filename: string, + public readonly original?: Error, + ) { + 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 const enum StashApplyErrorReason { WorkingChanges = 1, } export class StashApplyError extends Error { - static is(ex: any, reason?: StashApplyErrorReason): ex is StashApplyError { + static is(ex: unknown, reason?: StashApplyErrorReason): ex is StashApplyError { return ex instanceof StashApplyError && (reason == null || ex.reason === reason); } @@ -46,7 +64,7 @@ export const enum StashPushErrorReason { } export class StashPushError extends Error { - static is(ex: any, reason?: StashPushErrorReason): ex is StashPushError { + static is(ex: unknown, reason?: StashPushErrorReason): ex is StashPushError { return ex instanceof StashPushError && (reason == null || ex.reason === reason); } @@ -93,7 +111,7 @@ export const enum PushErrorReason { } export class PushError extends Error { - static is(ex: any, reason?: PushErrorReason): ex is PushError { + static is(ex: unknown, reason?: PushErrorReason): ex is PushError { return ex instanceof PushError && (reason == null || ex.reason === reason); } @@ -164,7 +182,7 @@ export const enum PullErrorReason { } export class PullError extends Error { - static is(ex: any, reason?: PullErrorReason): ex is PullError { + static is(ex: unknown, reason?: PullErrorReason): ex is PullError { return ex instanceof PullError && (reason == null || ex.reason === reason); } @@ -240,7 +258,7 @@ export const enum FetchErrorReason { } export class FetchError extends Error { - static is(ex: any, reason?: FetchErrorReason): ex is FetchError { + static is(ex: unknown, reason?: FetchErrorReason): ex is FetchError { return ex instanceof FetchError && (reason == null || ex.reason === reason); } @@ -301,7 +319,7 @@ export const enum WorktreeCreateErrorReason { } export class WorktreeCreateError extends Error { - static is(ex: any, reason?: WorktreeCreateErrorReason): ex is WorktreeCreateError { + static is(ex: unknown, reason?: WorktreeCreateErrorReason): ex is WorktreeCreateError { return ex instanceof WorktreeCreateError && (reason == null || ex.reason === reason); } @@ -343,7 +361,7 @@ export const enum WorktreeDeleteErrorReason { } export class WorktreeDeleteError extends Error { - static is(ex: any, reason?: WorktreeDeleteErrorReason): ex is WorktreeDeleteError { + static is(ex: unknown, reason?: WorktreeDeleteErrorReason): ex is WorktreeDeleteError { return ex instanceof WorktreeDeleteError && (reason == null || ex.reason === reason); }