Re: [PATCH] for-each-ref: delay parsing of --sort=<atom> options

Jeff King [off-list ref] writes:

Yeah, I faced something similar with 7c5045fc18 (ref-filter: apply
fallback refname sort only after all user sorts, 2020-05-03). I suspect
you could use the same keys as those tests, though I am OK if we simply
leave it as a quietly-fixed bug.

Ah, I guess I can cheat and add a new test after these.

If --no-sort weren't taking effect, the expected outcome would be
the asme as the previous step this copied from, but with --no-sort
clearing the sort keys, we sort by taggerdate and then tiebreak with
the refname, and taggeremail does not get into the picture (other
than being repeated at the end of the refname).

 t/t6300-for-each-ref.sh | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git c/t/t6300-for-each-ref.sh w/t/t6300-for-each-ref.sh
index 83921502a4..9f2c706c12 100755
--- c/t/t6300-for-each-ref.sh
+++ w/t/t6300-for-each-ref.sh

@@ -1024,6 +1024,27 @@ test_expect_success 'equivalent sorts fall back on refname' '
 	test_cmp expected actual
 '
 
+test_expect_success '--no-sort cancels the previous sort keys' '
+	cat >expected <<-\EOF &&
+	100000 <user1@example.com> refs/tags/multi-ref1-100000-user1
+	100000 <user2@example.com> refs/tags/multi-ref1-100000-user2
+	100000 <user1@example.com> refs/tags/multi-ref2-100000-user1
+	100000 <user2@example.com> refs/tags/multi-ref2-100000-user2
+	200000 <user1@example.com> refs/tags/multi-ref1-200000-user1
+	200000 <user2@example.com> refs/tags/multi-ref1-200000-user2
+	200000 <user1@example.com> refs/tags/multi-ref2-200000-user1
+	200000 <user2@example.com> refs/tags/multi-ref2-200000-user2
+	EOF
+	git for-each-ref \
+		--format="%(taggerdate:unix) %(taggeremail) %(refname)" \
+		--sort=-refname \
+		--sort=taggeremail \
+		--no-sort \
+		--sort=taggerdate \
+		"refs/tags/multi-*" >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'do not dereference NULL upon %(HEAD) on unborn branch' '
 	test_when_finished "git checkout main" &&
 	git for-each-ref --format="%(HEAD) %(refname:short)" refs/heads/ >actual &&

On Wed, Oct 20, 2021 at 02:32:21PM -0700, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

Yeah, I faced something similar with 7c5045fc18 (ref-filter: apply
fallback refname sort only after all user sorts, 2020-05-03). I suspect
you could use the same keys as those tests, though I am OK if we simply
leave it as a quietly-fixed bug.

Ah, I guess I can cheat and add a new test after these.

If --no-sort weren't taking effect, the expected outcome would be
the asme as the previous step this copied from, but with --no-sort
clearing the sort keys, we sort by taggerdate and then tiebreak with
the refname, and taggeremail does not get into the picture (other
than being repeated at the end of the refname).

Yeah, I think what you have here makes sense. It's too bad we can't run
it on the "before" state to double-check that we are triggering the old
breakage, but there is simply no "--no-sort" at all before your patch
(which is good, because it would have been broken ;) ).

-Peff