Re: [PATCH] builtin/push.c: add push.pushOption config

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

Stefan Beller <sbeller@google.com> writes:

>>> So in the config, we have to explicitly give an empty option to
>>> clear the previous options, but on the command line we do not need
>>> that, but instead we'd have to repeat any push options that we desire
>>> that were configured?
>>
>> It is not wrong per-se to phrase it like so, but I think that is
>> making it unnecessarily confusing by conflating two things.  (1)
>> configured values are overridden from the command line, just like
>> any other --option/config.variable pair
>
> because they are single value options (usually).
>
>> and (2) unlike usual single
>> value variables where "last one wins" rule is simple enough to
>> explain,, multi-value variables need a way to "forget everything we
>> said so far and start from scratch" syntax, especially when multiple
>> input files are involved.
>
> ok, my view of how that should be done is clashing once again
> with the projects established standards. Sorry for the noise.

I do not think it is a noise.  I was primarily focusing on "have to
explicitly" part, making it sound as if it was a flaw.  I do think
it is a good idea to explain a variable and/or an option is
multi-valued and how multiple instances of them interact with each
other.  I think the status quo is:

	Both configuration and command line, these multi-valued
	things accumulate.  The "command line can be used to
	override things from the configuration" rule applies as any
	other config/option pair.

	In addition, in the configuration, there is a mechanism to
	clear the values read so far with the "an empty value
	clears" rule, because you may not have control over, or it
	may be cumbersome to modify, lower-priority configuration
	files.  There is no such thing for command line, as the
	entirety of the command line for each invocation is under
	your control.

If somebody has

	[alias] mypush = push -ofoo

then the command line argument for one invocation of "git mypush"
may *not* be under your control and it might be also convenient if
there were a mechanism to clear what has been accumulated from the
command line.  It may be worth considering, but if we were going in
that direction, I suspect that it is probably a good idea to first
step back a bit and introduce a mechanism to easily define pairs of
option/config in a more sysmtematic way [*1*].  Once we have such a
mechanism, the "clear the previous" syntax for the command line
would be implemented uniformly in there.


[Footnote]

*1* E.g. right now, the fact that "push --push-option" and
    "push.pushOption" are related, or that "status -u<mode>" and
    "status.showUntrackedFiles" are related, is only known to the
    code and the documentation; I am not sure if it is even a good
    idea to add config to each and every option that exists, but for
    the ones that do exist, it would be nicer if there were a more
    uniform and systematic way for parse-options.c and config.c APIs
    to help our code, instead of writing git_config() callback and
    options[] array to give to parse_options(), making sure they
    refer to the same internal variable, and the latter overrides
    the former.