On Wed, Apr 04, 2018 at 03:57:12AM -0400, Eric Sunshine wrote: > On Wed, Apr 4, 2018 at 2:07 AM, Taylor Blau <m...@ttaylorr.com> wrote: > > [...] > > In fact, `git config` will not accept multiple type specifiers at a > > time, as indicated by: > > > > $ git config --int --bool some.section > > error: only one type at a time. > > > > This patch uses `OPT_SET_INT` to prefer the _last_ mentioned type > > specifier, so that the above command would instead be valid, and a > > synonym of: > > > > $ git config --bool some.section > > > > This change is motivated by two urges: (1) it does not make sense to > > represent a singular type specifier internally as a bitset, only to > > complain when there are multiple bits in the set. `OPT_SET_INT` is more > > well-suited to this task than `OPT_BIT` is. (2) a future patch will > > introduce `--type=<type>`, and we would like not to complain in the > > following situation: > > > > $ git config --int --type=int > > I can understand "last wins" for related options such as "--verbose > --quiet" or "--verbose=1 --verbose=2", but I have a lot of trouble > buying the "last wins" argument in this context where there is no > semantic relationship between, say, --bool and --expiry-date. > Specifying conflicting options together is likely to be unintentional, > thus reporting it as an error seems sensible, so I'm not convinced > that patch's removal of that error is a good idea. > > 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. Thanks, Taylor
