On Tue, Mar 6, 2018 at 1:52 AM, Jeff King <[email protected]> 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
