Re: [PATCH] builtin/config.c: prefer `--type=bool` over `--bool`, etc.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Mar 30, 2018 at 09:00:05AM -0700, Junio C Hamano wrote:

> Jeff King <peff@xxxxxxxx> writes:
> 
> > ... But actually, last-one-wins applies only
> > to a _single_ option, not necessarily unrelated ones. Many other
> > multi-action commands actually have a series of separate boolean flags,
> > and then complain when more than one of the flags is set.
> >
> > So maybe it's not such a good idea for the actions (I do still think
> > it's the right path for the types).
> 
> If this were using command verbs (e.g. "git config get foo.bar") as
> opposed to command options (e.g. "git config --get foo.bar"), it
> wouldn't ahve allowed multiple command verbs from the command line,
> and last-one-wins would not have made much sense because there is no
> way to trigger it as a desirable "feature".
> 
> Just like the topic of the discussion unifies --int/--bool/etc. into
> a single --type={int,bool,...}, perhaps the existing command options
> --get/--list/etc. can be taken as if they were a mistaken historical
> way to spell --action={get,list,...}.  I of course am not recommending
> to add a new "--action" option.  I am suggesting it as a thought-aid
> to see if actions are all that different from value type options.
> 
> I agree that a-bit-per-type that is checked with HAS_MULTI_BITS()
> for error at the end does not make much sense.  I also think what
> you did in this patch for actions is a good clean-up for the above
> reason.

I agree the code internally is nicer after the patch. If we throw away
the argument of "last-one-wins is more consistent with other parts of
git" (because I don't really think that it is for this type of option),
I could possibly buy this as a code cleanup. But it does have a
user-visible impact, which makes the question: are we _OK_ with
switching to last-one-wins?

-Peff



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux