Re: git silently ignores include directive with single quotes

From: Jeff King <peff@peff.net>
Date: Sep 8, 2018, 21:14 UTC
Message-ID: <20180908211436.GA31560@sigill.intra.peff.net>
In-Reply-To: <87bm97rcih.fsf@evledraar.gmail.com>

On Sat, Sep 08, 2018 at 09:54:14PM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 12 quoted lines

> The reason missing includes are ignored is that the way this is expected
> to be used is e.g.:
> 
>     [include]
>         path ~/.gitconfig.work
> 
> Where .gitconfig.work is some configuration you're going to drop into
> place on your $dayjob servers, but not on your personal machine, even
> though you sync the same ~/.gitconfig everywhere.
> 
> A lot of people who use includes rely on this, but I see from this
> thread this should be better documented.

Right, this was an intentional choice at the time the feature was added, to support this kind of feature. I'd note also that it mirrors other misspelled keys. E.g.:

  [include]
  psth = whatever

will also not generate an error. This is also intentional, for two reasons:

  1. Git's config format has always been designed to carry extra keys
     used by third-party scripts and porcelain. So we don't actually
     know the complete set of valid keys. (Though you could make an
     argument that git-core could stake out include.* as its own).

  2. It makes using multiple git versions easier in some ways (though
     also harder in others). A config key that isn't known to the
     current version will be quietly ignored.

Of course those things mean that true spelling mistakes are harder to catch as such, because Git doesn't know that's what they are. And here I'm talking config _keys_, not values. So I'm just explaining the philosophical thinking that led to the "missing file is a silent noop". It doesn't _have_ to behave the same.

That said, it _does_ behave the same and people are likely depending on it at this point. So if we introduce a warning, for example, there needs to be some way to suppress it.

Probably:

  [include]
  warnOnMissing = false
  path = ...

would be enough (with the default being "true").

You could even do:

  [include]
  warnOnMissing = false
  path = one
  warnOnMissing = true
  path = two

to treat two includes differently (though I'm not sure why you would want to).

Show 10 quoted lines

> If we were to make nonexisting files an error, we'd need something like
> an extension of the includeIf syntax added in 3efd0bedc6 ("config: add
> conditional include", 2017-03-01) 3efd0bedc6 ("config: add conditional
> include", 2017-03-01). I.e.:
> 
>     [includeIfcond "test -e ~/.gitconfig.work"]
>         path = ~/.gitconfig.work
> 
> Or something like that, this is getting increasingly harder to shove
> into the *.ini config syntax.

I think it would be simpler to just introduce a new key that's a variant of "path". Like:

  [include]
  maybePath = ~/.gitconfig.work

Though if it really is just a warning, the "warnOnMissing" above would make that unnecessary (and it also scales better if we have to end up adding more behavior tweaks in the future).

-Peff