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

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> 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.

A bit more ambitious might want to consider another more generally
applicable avenue, which would help the userbase a lot more, before
continuing.

We start from the realization that this is not the only
configuration variable that specifies a filename that could be
missing.  There may be other variables that name files to be used
("git config --help" would hopefully be the most comprehensive, but
"git grep -e git_config_pathname \*.c" would give us quicker
starting point to gauge how big an impact to the system we would be
talking about).

What do the codepaths that use these variables do when they find
that the named files are missing?  Do some of them die, some
others just warn, and yet some others silently ignore?  Would such
an inconsistency hurt our users?

Among the ones that die, are there ones that could reasonably
continue as if the configuration variable weren't there and no file
was specified (i.e. similar to what you want blame.ignoreRevsFile to
do)?  Among the ones that are silently ignored, are there ones that
may benefit by having a typo-detection?  Do all of them benefit if
the behaviour upon missing files can be configurable by the end-user?

Depending on the answers to the above questions, it might be that it
is not a desirable approach to add "blame.ignoreRevsFileIsOptional"
configuration variable, as all the existing configuration variables
that name files would want to add their own.  We might be better off
inventing a syntax for the value of blame.ignoreRevsFile (and other
variables that name files) to mark if the file is optional (i.e.
silently ignore if the named file does not exist) or required (i.e.
diagnose as a configuration error).  For example, we may borrow from
the "magic" syntax for pathspecs that begin with ":(", with comma
separated "magic" keywords and ends with ")" and specify optional
pathname configuration like so:

    [blame] ignoreRevsFile = :(optional).gitignorerevs

and teach the config parser to pretend as if it saw nothing when it
notices that the named file is missing.  That approach would cover
not just this single variable, but other variables that are parsed
using git_config_pathname() may benefit the same way (of course, the
callsites for git_config_pathmame() must be inspected and adjusted
for this to happen).

Thanks.