Re: [PATCH] config: teach "git -c" to recognize an empty string

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 4, 2014, 22:25 UTC
Message-ID: <xmqq61i7riy4.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20140804215644.GA21510@peff.net>

Jeff King <peff@peff.net> writes:

> This is technically a backwards incompatibility, but I'd consider it a
> simple bugfix. The existing behavior was unintentional, made no sense,
> and was never documented.

Yeah, I tend to agree. I actually would not shed any tears if the breakage were that it was impossible to pass "NULL is true" boolean via "git -c" interface, but it is the other way around. It is much more grave a problem that we cannot pass an empty string as a value, and we should fix it.

Show 5 quoted lines

> Looking over strbuf_split's interface, I think it's rather
> counter-intuitive, and I was tempted to change it. But there are several
> other callers that rely on it, and the chance for introducing a subtle
> bug is high. This is the least invasive fix (and it really is not any
> less readable than what was already there :) ).

;-)

Show 10 quoted lines

> +# We just need a type-specifier here that cares about the
> +# distinction internally between a NULL boolean and a real
> +# string (because most of git's internal parsers do care).
> +# Using "--path" works, but we do not otherwise care about
> +# its semantics.
> +test_expect_success 'git -c can represent empty string' '
> +	echo >expect &&
> +	git -c foo.empty= config --path foo.empty >actual &&
> +	test_cmp expect actual
> +'

Another way may be "git config -l" and see if we see a = on the entry for foo.empty, but I think the way you did this is nicer.

>  test_expect_success 'key sanity-checking' '
>  	test_must_fail git config foo=bar &&
>  	test_must_fail git config foo=.bar &&