Re: How to undo previously set configuration? (again)

From: Jeff King <peff@peff.net>
Date: Apr 30, 2019, 21:14 UTC
Message-ID: <20190430211415.GB16290@sigill.intra.peff.net>
In-Reply-To: <CACsJy8B5j2K=RU7N+h-i9HszuYuYd+fNmAftVDW-4nJ_o5z8Sw@mail.gmail.com>

On Fri, Apr 26, 2019 at 04:36:40PM +0700, Duy Nguyen wrote:

Show 13 quoted lines

> > I'm confused.  Isn't that bog-standard Git usage, not a custom hack?
> > That is, I thought the intended behavior is always
> >
> >  1. For single-valued options, last value wins.
> >  2. For multi-valued options, empty clears the list.
> 
> I didn't know this! Should it be documented? At least a quick skimming
> through config.txt does not mention anything about empty value
> clearing multi-valued options.
> 
> I also wanted to see if it's true. However, the first var I checked,
> branch.*.merge does not follow this rule. I got disappointed and
> stopped.

It's definitely not implemented universally; each consumer of the config option must decide on it (and it will probably always be that way to some degree, since we don't know the semantics of each options; recall that we may be holding config keys for other non-core programs, too). And we just haven't retro-fitted a lot of those older options because nobody has been bothered by it.

That said, I am a proponent of having some kind of clearing mechanism (and I was the one who added credential.helper's mechanism, which has been mentioned in this thread). I think it makes things a lot less difficult if we don't have to change the syntax of the config files to do it. With that constraint, that pretty much leaves:

  1. Some sentinel value like the empty string. That one _probably_
     works in most cases, but there may be lists which want to represent
     the empty value. There could be other sentinel values (e.g.,
     "CLEAR") which are simply unlikely to be used as real values.

  2. The boolean syntax (i.e., "[foo]bar" with no equals) is almost
     always bogus for a list. So that can work as a sentinel that is
     OK syntactically.

Show 8 quoted lines

> > Which of course leaves room for improvement in documentation and how
> > we organize the implementation (as Peff discussed in [1]), but isn't
> > it nice to already have something in place that doesn't require
> > inventing a new syntax?
> 
> This cannot undefine a variable though, especially those single-valued
> ones. But I think for most cases, the user just needs to find out what
> is the default value and set to that one.

Having "default" is a little more convenient, but it would need to be implemented on a per-value basis anyway. In many cases the caller of the config code has implemented last-one-wins by overwriting an old value, and only it knows how to restore "default".

I guess if we moved completely to a configset world, then asking for git_config_get_string() could resolve "default" completely. But we are a ways from that.

-Peff