Re: [PATCH 1/2] ref-filter: apply --ignore-case to all sorting keys

From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: clime <clime7@gmail.com>, Git List <git@vger.kernel.org>
Subject: Re: [PATCH 1/2] ref-filter: apply --ignore-case to all sorting keys
Date: Mon, 04 May 2020 14:00:12 -0700	[thread overview]
Message-ID: <xmqqh7wvfmar.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <20200503091157.GA170902@coredump.intra.peff.net> (Jeff King's message of "Sun, 3 May 2020 05:11:57 -0400")

Jeff King <peff@peff.net> writes:

> would sort the primary key (taggername) case-insensitively, but sort the
> refname case-sensitively. We have two options here:
>
>   - teach callers to set ignore_case on the whole list
>
>   - replace the ref_sorting list with a struct that contains both the
>     list of sorting keys, as well as options that apply to _all_
>     keys
>
> I went with the first one here, as it gives more flexibility if we later
> want to let the users set the flag per-key (presumably through some
> special syntax when defining the key; for now it's all or nothing
> through --ignore-case).

A good design decision I would fully support.

> +test_expect_success 'for-each-ref --ignore-case works on multiple sort keys' '
> +	# name refs numerically to avoid case-insensitive filesystem conflicts

Very considerate.  If I were writing these nested loops, I am sure I
would have used "tag-$email-$subject" to be cute.

Queued.  Thanks.

> +	nr=0 &&
> +	for email in a A b B
> +	do
> +		for subject in a A b B
> +		do
> +			GIT_COMMITTER_EMAIL="$email@example.com" \
> +			git tag -m "tag $subject" icase-$(printf %02d $nr) &&
> +			nr=$((nr+1))||
> +			return 1
> +		done
> +	done &&
> +	git for-each-ref --ignore-case \
> +		--format="%(taggeremail) %(subject) %(refname)" \
> +		--sort=refname \
> +		--sort=subject \
> +		--sort=taggeremail \
> +		refs/tags/icase-* >actual &&
> +	cat >expect <<-\EOF &&
> +	<a@example.com> tag a refs/tags/icase-00
> +	<a@example.com> tag A refs/tags/icase-01
> +	<A@example.com> tag a refs/tags/icase-04
> +	<A@example.com> tag A refs/tags/icase-05
> +	<a@example.com> tag b refs/tags/icase-02
> +	<a@example.com> tag B refs/tags/icase-03
> +	<A@example.com> tag b refs/tags/icase-06
> +	<A@example.com> tag B refs/tags/icase-07
> +	<b@example.com> tag a refs/tags/icase-08
> +	<b@example.com> tag A refs/tags/icase-09
> +	<B@example.com> tag a refs/tags/icase-12
> +	<B@example.com> tag A refs/tags/icase-13
> +	<b@example.com> tag b refs/tags/icase-10
> +	<b@example.com> tag B refs/tags/icase-11
> +	<B@example.com> tag b refs/tags/icase-14
> +	<B@example.com> tag B refs/tags/icase-15
> +	EOF
> +	test_cmp expect actual
> +'
> +
>  test_done