Taylor Blau <m...@ttaylorr.com> writes: > Internally, we represent `git config`'s type specifiers as a bitset > using OPT_BIT. 'bool' is 1<<0, 'int' is 1<<1, and so on. This technique > allows for the representation of multiple type specifiers in the `int > types` field, but this multi-representation is left unused. > > 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
The above does not exactly argue for adopting the last-one-wins semantics, and still leaves it unclear if we want to complain against $ git config --bool --type=int Is it intentionally left vague if we want to (or not want to) complain when such a conflicting specification is given? We could keep the traditional behaviour of "only one type at a time" error and still move away from the bitset representation that does not make sense, if we wanted to. Initialize the "type" variable to an unset value, and use a callback to ensure either the variable is set to the unset value, or the value being set is already in the variable. I think if you use OPT_CMDMODE(), it would do all of that for you automatically. I suspect that it may be OK to switch to last-one-wins, but then we should give a justification that is a bit stronger than "we want to avoid complaining against --int --type=int" (i.e. "we want to switch to last-one-wins for such and such reasons"). > diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh > index 4f8e6f5fde..24de37d544 100755 > --- a/t/t1300-repo-config.sh > +++ b/t/t1300-repo-config.sh > @@ -1611,4 +1611,15 @@ test_expect_success '--local requires a repo' ' > test_expect_code 128 nongit git config --local foo.bar > ' > > +cat >.git/config <<-\EOF && > +[core] > +number = 10 > +EOF > + > +test_expect_success 'later legacy specifiers are given precedence' ' > + git config --bool --int core.number >actual && > + echo 10 >expect && > + test_cmp expect actual > +' And this expects more than we gave justifications for in the proposed log message. I do not think it is necessarily a bad idea to switch to last-one-wins, but if that is where we really want to go, the proposed log message is being misleading. It is true that OPT_SET_INT is more suited to complain when two conflicting things are given than OPT_BIT, but this example actually tells us that you no longer want to catch an error to give conflicting requests.