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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 26, 2019, 02:13 UTC
Message-ID: <xmqqmukdwm32.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20190425143614.GA91608@google.com>

Jonathan Nieder <jrnieder@gmail.com> writes:

> I'm confused.  Isn't that bog-standard Git usage, not a custom hack?

Depends on the definition of "hack".

The only native provision at the config API level that lets the calling programs realize the "desired behaviour" you listed below without doing anything special is "for a single-valued one, the last one wins", for which the newer style config_get_value() will give you the "last one wins" semantics. With the original style parsing using the git_config() with callback still needs to do the "last one wins" by the caller of the config API.

The "empty clears the list" for multi-valued option is totally up to the application. Even the newer configset__get_value_multi() API function does not reset a list to empty when it sees an empty string. The caller gets a list of value strings that happens to have an empty string in the middle.

So, I think the "bog-standard" Git convention must be maintained by each application codepath to implement it with a custom hack.

Perhaps you did not mean with "hack" a "hacky implementation", but a "hacky design". If that is the case, yeah, I agree with you that the items #1 and #2 below are what we try to make sure our programs follow; I am not sure about #3 myself, as I do not think offhand a good example of it, though.

Show 9 quoted lines

> 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.
>  3. When there is a special behavior triggered by not supplying the
>     option at all, offer an explicit value like "default" that triggers
>     the same behavior, too.
>
> and that any instance of a command that isn't following that is a bug.

If we make that declaration, we could enforce the "empty clears the list" at the API level when the configset API is in use.