Thread (2 messages) flat view 2 messages, 2 authors, 2017-10-23

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

From: Junio C Hamano <hidden>
Date: 2017-10-21 01:52:41

Possibly related (same subject, not in this thread)

Stefan Beller [off-list ref] writes:
quoted
quoted
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).
quoted
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help