Re: [PATCH] config: fix case sensitive subsection names on writing

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 28, 2018, 01:01 UTC
Message-ID: <xmqqlg9wjhq7.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180727233606.179965-1-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

> A use reported a submodule issue regarding strange case indentation

s/use/&r/;  Is this "indentation" issue?

> issues, but it could be boiled down to the following test case:
> ...

Show 13 quoted lines

> +test_expect_success 'old-fashioned settings are case insensitive' '
> +	test_when_finished "rm -f testConfig testConfig_expect testConfig_actual" &&
> +
> +	cat >testConfig <<-EOF &&
> +		# insensitive:
> +		[V.A]
> +		r = value1
> +	EOF
> +	q_to_tab >testConfig_expect <<-EOF &&
> +		# insensitive:
> +		[V.A]
> +		Qr = value2
> +	EOF

It is unfortunate that we hardcode the exact indentation in the test to make it care. Perhaps a wrapper around test_cmp that is used locally in this file to first strip the leading HT from both sides of the comparison would make it more robust?

Show 7 quoted lines

> +	for key in "v.a.r" "V.A.r" "v.A.r" "V.a.r"
> +	do
> +		cp testConfig testConfig_actual &&
> +		git config -f testConfig_actual v.a.r value2 &&
> +		test_cmp testConfig_expect testConfig_actual
> +	done
> +'

I think you meant to use "$key" when setting the variable to value2.

When the test_cmp fails with "v.a.r" but later succeeds and most importantly succeeds with "V.a.r" (i.e. the last one), wouldn't the whole thing suceed? I think the common trick people use is to end the last one with "|| return 1" in a loop inside test_expect_success.