Re: [PATCH 0/1] blame: Skip missing ignore-revs file

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 8, 2021, 05:43 UTC
Message-ID: <xmqqtuk0h4ph.fsf@gitster.g>
In-Reply-To: <CADm0i3-ToKo1gNTXXLHH6i2d4qpz771VeRjDsfJjgbgMfhx6rA@mail.gmail.com>

Noah Pendleton <noah.pendleton@gmail.com> writes:

Show 6 quoted lines

> Very good point about no longer catching misconfiguration. For
> detecting provenance of a setting, I think we'd need to tag the config
> options with it when they're loaded, possibly in 'struct
> config_set_element' or similar. What do you think about instead
> emitting a warning message on stderr in the case of misconfiguration,
> but still continuing? Eg:

Unconditionally continuing with just a warning would not be a good approach for at least two reasons. (1) the user may truly have intended that ignoreRevsFile to be optional, in which case the warning is a nuisance that does not add any value, and (2) it truly may be a misconfiguration that the named file did not exist, but the output from "git blame" will wipe the display and the warning would very well go unnoticed (or more likely the user may notice that there was a warning, but it will go away before the user has a chance to really read it, which is a lot worse and frustrating experience).

I think an easier way out is to introduce a new configuration variable blame.ignoreRevsFileIsOptional which takes a boolean value, and when it is set to true, silently ignore when the named file does not exist without any warning. When the variable is set to false (or the variable does not exist), we can keep the current behaviour of noticing a misconfigured blame.ignoreRevsFile and error out.

That way, the current users who rely on the typo detection feature can keep relying on it, and those who want to make it optional can do so without getting annoyed by a warning.