Re: [PATCH 1/4] builtin/config: introduce `--default`
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Mar 6, 2018, 07:14 UTC
- Message-ID
- <CAPig+cSMx8oSWvT2dGgz-DwfLJkGDUFP-Gs19c=izK-=3422JA@mail.gmail.com>
- In-Reply-To
- <20180306065208.GB3522@sigill.intra.peff.net>
On Tue, Mar 6, 2018 at 1:52 AM, Jeff King <peff@peff.net> wrote:
Show 31 quoted lines
> On Mon, Mar 05, 2018 at 06:17:26PM -0800, Taylor Blau wrote: >> In an aim to replace: >> $ git config --get-color slot [default] [...] >> with: >> $ git config --default default --color slot [...] >> introduce `--defualt` to behave as if the given default were present and >> assigned to slot in the case that that slot does not exist. > > I think this motivation skips over the beginning part of the story, > which is why we want "--color --default". :) > > IMHO, the reason we want --default is two-fold: > > 1. Callers have to handle parsing defaults themselves, like: > > foo=$(git config core.foo || echo 1234) > > For an integer, that's not too bad, since you can write "1048576" > instead of "1M". For colors, it's abominable, which is why we added > "--get-color". But as we add more types that are hard to parse > (like --expiry-date), it would be nice for them to get the same > defaulting feature without adding --get-expiry-date, etc. > > 2. --get-color is a one-off unlike all of the other types. That's bad > interface design, but the inconsistency also makes it harder to add > features which treat the types uniformly (like, say, a --stdin > query mode). > > And perhaps minor, but it's also easier to correctly error-check > --default, since the "foo" example above would do the wrong thing if > git-config encountered a fatal error.
Thanks. For someone (me) who didn't follow the earlier discussion closely, this motivating explanation really helps; especially since the commit message mentions only color, which seems to already allow for a default value, so it wasn't clear what benefit the new --default provides.
Show 21 quoted lines
>> +test_expect_success 'marshals default value as bool-or-int' '
>> + echo "1
>> +true" >expect &&
>> + git config --default 1 --bool-or-int core.foo >actual &&
>> + git config --default true --bool-or-int core.foo >>actual &&
>> + test_cmp expect actual
>> +'
>
> Funny indentation. Use:
>
> {
> echo 1 &&
> echo true
> } >expect &&
>
> or
>
> cat >expect <<-\EOF
> 1
> true
> EOFOr, simpler:
test_write_lines 1 true >expect