Re: [PATCH v3 1/2] builtin/config.c: treat type specifiers singularly

[Jeff King <peff@peff.net>]

On Wed, Apr 04, 2018 at 06:53:04PM -0700, Taylor Blau wrote:

> > I understand that you're doing this to avoid complaining about "--int
> > --type=int", but exactly how that case is supported should be an
> > implementation detail; it doesn't need to bleed into the UI as an
> > unnecessary and not-well-justified loosening of an age-old
> > restriction. There are other ways to support "--int --type=int"
> > without "last wins". Also, do we really need to support "--int
> > --type=int"? Is that an expected use-case?
> 
> This is a fair concern, though I view the problem slightly differently.
> I see this change as being motivated by the fact that we are adding
> "--type", not removing an age-old restriction.
> 
> Peff's motivation for this--as I understand it--is that "--type=int"
> should be a _true_ synonym for "--int". Adopting the old-style
> "OPT_SET_BIT" for this purpose makes "--type=int" and "--int" _mostly_ a
> synonym for one another, except that "--type=bool --type=int" will not
> complain, whereas "--bool --int" would.
> 
> Loosening this restriction, in my view, brings us closer (1) to the new
> semantics of multiple "--type"'s, and (2) to the existing semantics of
> "--verbose=1 --verbose=2" as you mentioned above.
> 
> I would like to hear Peff's take on this as well, since he suggested the
> patch originally, and would likely provide the clearest insight into
> this.

I think you've captured it fairly well.  The options _are_ semantically
linked, in that they are all mutually-exclusive types. Obviously we
could continue to flag errors, and even catch "--type=int --type=bool"
in the same way if we really wanted to (by using a custom parse-options
callback).

So I think the primary value here is in the code cleanup. Even without
the new "--type" option, avoiding the bitset makes the code clearer
(IMHO).

I do agree that a user saying "--int --bool" is almost certainly an
error, and they'd be just as happy to see an error message as to get the
last-one-wins behavior. But I also doubt that it comes up very much
either way.

-Peff