Re: git silently ignores include directive with single quotes

[Jeff King <peff@peff.net>]

On Sun, Sep 09, 2018 at 12:32:57AM +0200, Ævar Arnfjörð Bjarmason wrote:

> > 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).
> 
> I think this is introducing a brand new caveat into our *.ini syntax,
> i.e. that we're sensitive to the order in which we're parsing
> *different* keys.
> 
> I.e. we already had the concept that some keys override existing sets
> (e.g. user.name), but not that a x.y=foo controls the behavior of a
> subsequent a.b=bar, or the other way around.

This already exists. For example:

  echo '[foo]bar = inc' >config.inc
  echo '[foo]bar = main' >config.main
  echo '[include]path = config.inc' >>config.main
  git config -f config.main --includes foo.bar

Arriving at that answer requires expanding the include's contents at
the exact same spot in the file.

As far as I know this is the only case, though, so include.path really
is special. And this would be expanding that specialness to other things
in include.*. But I'm not sure that is really that big a deal. That
warnOnMissing isn't meant to be just a normal key. It's an
order-sensitive directive for further parsing, just like include.path
is. Either the parser understands includes (and has to handle these) or
it doesn't.

So I'm not worried about any burden on the parsing side, but...

> This also makes programmatic (via "git config") editing of the config
> hard, we'd need to introduce something like:
> 
>     git config -f ~/.gitconfig a.b bar
>     git config -f ~/.gitconfig --before a.b x.y foo
> 
> To set a.b=bar before x.y=foo, or --after or whatever.

Yes, I don't think "git config include.warnOnMissing true" would be very
useful, because it would generally be added after any includes you have,
and therefore not affect them.

I think this is generally an issue with include.path, too. My assumption
has been that anybody at the level of using includes is probably going
to be hand-editing anyway.

But if include.* is special on the parsing side, I don't know that it is
that bad to make it special on the writing side, too. I.e., to recognize
"git config include.warnOnMissing true" and always add it at the head of
any existing include block.

It certainly _feels_ hacky, but I think it would behave sensibly and
predictably. And it would just work, as opposed to requiring something
like "--before", which would be quite a subtle gotcha for somebody to
forget to use.

> Yeah, we could do that, and it wouldn't break the model described above,
> We can make that work, but this would be nasty. E.g. are we going to
> treat EACCES and ENOENT the same way in this construct?

I don't have a strong opinion (after all, I already wrote the behavior I
thought was reasonable long ago ;) ). So I think it would be up to
somebody to propose. We do already report and die on EACCES (and
basically any other error except ENOENT). So if we did treat them both
as a warning, that would be a weakening for EACCES.

-Peff