Web lists-archives.com

Re: [PATCH 1/4] builtin/config: introduce `--default`




On Tue, Mar 6, 2018 at 1:52 AM, Jeff King <peff@xxxxxxxx> wrote:
> 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.

>> +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
>   EOF

Or, simpler:

    test_write_lines 1 true >expect