Re: [PATCH v5 2/2] config: improve diagnostic for "set" with missing value

[Junio C Hamano <gitster@pobox.com>]

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
> index 11fc976f3a..ed122d1100 100755
> --- a/t/t1300-config.sh
> +++ b/t/t1300-config.sh
> @@ -469,6 +469,61 @@ test_expect_success 'invalid key' '
>  	test_must_fail git config inval.2key blabla
>  '
>  
> +test_expect_success 'set with 1 arg of "key=value": valid key suggests split form' '
> +	test_must_fail git config set pull.rebase=false 2>err &&
> +	test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
> +	test_grep "did you mean .git config set pull\\.rebase false." err
> +'

This is a syntax error of the command line, but the lhs of '=' makes
us suspect that the user may have meant to assign to that variable.
Makes perfect sense.

> +test_expect_success 'set with 1 arg of "key=value": implicit form suggests split form' '
> +	test_must_fail git config pull.rebase=false 2>err &&
> +	test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
> +	test_grep "did you mean .git config set pull\\.rebase false." err
> +'

Ditto, the syntax may be an implicit "get" with bogus variable name,
or an implicit "set" with variable name and its value concatenated
into one argument with '='.  The message seems to be assuming the
latter, which is OK to me.

> +test_expect_success 'set with 1 arg of "key=value": invalid key does not suggest split form' '
> +	test_must_fail git config set foo=bar 2>err &&
> +	test_grep "missing value to set to a variable with an invalid name .foo=bar." err &&
> +	test_grep ! "did you mean" err
> +'

OK.

> +test_expect_success 'set with 1 arg: variable name starting with digit is invalid' '
> +	test_must_fail git config set foo.1bar=baz 2>err &&
> +	test_grep "missing value to set to a variable with an invalid name .foo\\.1bar=baz." err &&
> +	test_grep ! "did you mean" err
> +'

OK.  The above two should always give us the same error (except for
the actual bogus names given by the user to the command).

> +test_expect_success 'set with 1 arg: digit-led section name is valid' '
> +	test_must_fail git config set 1foo.bar=baz 2>err &&
> +	test_grep "missing value to set to the variable .1foo\\.bar=baz." err &&
> +	test_grep "did you mean .git config set 1foo\\.bar baz." err
> +'

OK.

> +test_expect_success 'set with 1 arg: subsection plus invalid variable name' '
> +	test_must_fail git config set foo.some.b_r=baz 2>err &&
> +	test_grep "missing value to set to a variable with an invalid name .foo\\.some\\.b_r=baz." err &&
> +	test_grep ! "did you mean" err
> +'

This is the third one that should be identical to earlier two that
gave a bogus variable name.

> +test_expect_success 'set with 1 arg of valid key reports missing value' '
> +	test_must_fail git config set pull.rebase 2>err &&
> +	test_grep "missing value to set to the variable .pull\\.rebase." err &&
> +	test_grep ! "did you mean" err
> +'

Did we see this already?  No, this is different from the earlier one
that had "=false".  This is a bog standard "you said set but did not
say what value to set to".  Good.

> +test_expect_success 'set with 2 args including "=" in invalid key does not suggest' '
> +	test_must_fail git config set pull.rebase=false true 2>err &&
> +	test_grep ! "did you mean" err
> +'

OK.  Do we want to see that the bogus variable name reported?

> +test_expect_success '"=" inside subsection is valid' '
> +	test_when_finished "rm -f subsection.cfg" &&
> +	git config set -f subsection.cfg foo.bar=baz.boo qux &&
> +	echo qux >expect &&
> +	git config get -f subsection.cfg foo.bar=baz.boo >actual &&
> +	test_cmp expect actual
> +'

Excellent.