git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: clime <clime7@gmail.com>
Cc: Git List <git@vger.kernel.org>
Subject: [PATCH 1/2] ref-filter: apply --ignore-case to all sorting keys
Date: Sun, 3 May 2020 05:11:57 -0400	[thread overview]
Message-ID: <20200503091157.GA170902@coredump.intra.peff.net> (raw)
In-Reply-To: <20200503090952.GA170768@coredump.intra.peff.net>

All of the ref-filter users (for-each-ref, branch, and tag) take an
--ignore-case option which makes filtering and sorting case-insensitive.
However, this option was applied only to the first element of the
ref_sorting list. So:

  git for-each-ref --ignore-case --sort=refname

would do what you expect, but:

  git for-each-ref --ignore-case --sort=refname --sort=taggername

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).

The new test covers this by sorting on both tagger and subject
case-insensitively, which should compare "a" and "A" identically, but
still sort them before "b" and "B". We'll break ties by sorting on the
refname to give ourselves a stable output (this is actually supposed to
be done automatically, but there's another bug which will be fixed in
the next commit).

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/branch.c        |  2 +-
 builtin/for-each-ref.c  |  2 +-
 builtin/tag.c           |  2 +-
 ref-filter.c            |  6 ++++++
 ref-filter.h            |  2 ++
 t/t6300-for-each-ref.sh | 40 ++++++++++++++++++++++++++++++++++++++++
 6 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index d8297f80ff..86341cc835 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -739,7 +739,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		 */
 		if (!sorting)
 			sorting = ref_default_sorting();
-		sorting->ignore_case = icase;
+		ref_sorting_icase_all(sorting, icase);
 		print_ref_list(&filter, sorting, &format);
 		print_columns(&output, colopts, NULL);
 		string_list_clear(&output, 0);
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 465153e853..57489e4eab 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -70,7 +70,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 
 	if (!sorting)
 		sorting = ref_default_sorting();
-	sorting->ignore_case = icase;
+	ref_sorting_icase_all(sorting, icase);
 	filter.ignore_case = icase;
 
 	filter.name_patterns = argv;
diff --git a/builtin/tag.c b/builtin/tag.c
index dd160b49c7..ff7610b5c8 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -485,7 +485,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	}
 	if (!sorting)
 		sorting = ref_default_sorting();
-	sorting->ignore_case = icase;
+	ref_sorting_icase_all(sorting, icase);
 	filter.ignore_case = icase;
 	if (cmdmode == 'l') {
 		int ret;
diff --git a/ref-filter.c b/ref-filter.c
index 35776838f4..bdb3535ce5 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2317,6 +2317,12 @@ static int compare_refs(const void *a_, const void *b_, void *ref_sorting)
 	return 0;
 }
 
+void ref_sorting_icase_all(struct ref_sorting *sorting, int flag)
+{
+	for (; sorting; sorting = sorting->next)
+		sorting->ignore_case = !!flag;
+}
+
 void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)
 {
 	QSORT_S(array->items, array->nr, compare_refs, sorting);
diff --git a/ref-filter.h b/ref-filter.h
index 64330e9601..8ecc33cdfa 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -114,6 +114,8 @@ void ref_array_clear(struct ref_array *array);
 int verify_ref_format(struct ref_format *format);
 /*  Sort the given ref_array as per the ref_sorting provided */
 void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);
+/*  Set the ignore_case flag for all elements of a sorting list */
+void ref_sorting_icase_all(struct ref_sorting *sorting, int flag);
 /*  Based on the given format and quote_style, fill the strbuf */
 int format_ref_array_item(struct ref_array_item *info,
 			  const struct ref_format *format,
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index b3c1092338..c9caf26327 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -895,4 +895,44 @@ test_expect_success 'for-each-ref --ignore-case ignores case' '
 	test_cmp expect actual
 '
 
+test_expect_success 'for-each-ref --ignore-case works on multiple sort keys' '
+	# name refs numerically to avoid case-insensitive filesystem conflicts
+	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
-- 
2.26.2.957.g6dc93e954a


  reply	other threads:[~2020-05-03  9:12 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-02 20:31 git for-each-ref - sorting by multiple keys clime
2020-05-03  9:09 ` Jeff King
2020-05-03  9:11   ` Jeff King [this message]
2020-05-03 11:44     ` [PATCH 1/2] ref-filter: apply --ignore-case to all sorting keys Danh Doan
2020-05-04 15:13       ` Jeff King
2020-05-04 15:37         ` Junio C Hamano
2020-05-04 16:07           ` [PATCH] CodingGuidelines: drop arithmetic expansion advice to use "$x" Jeff King
2020-05-04 16:28             ` Carlo Marcelo Arenas Belón
2020-05-04 16:33               ` Jeff King
2020-05-04 19:47                 ` Junio C Hamano
2020-05-04 23:32             ` Danh Doan
2020-05-05 20:40             ` Junio C Hamano
2020-05-05 21:07               ` Jeff King
2020-05-05 21:30                 ` Junio C Hamano
2020-05-04 21:00     ` [PATCH 1/2] ref-filter: apply --ignore-case to all sorting keys Junio C Hamano
2020-05-05  0:11       ` Jeff King
2020-05-05  0:13     ` Taylor Blau
2020-05-03  9:13   ` [PATCH 2/2] ref-filter: apply fallback refname sort only after all user sorts Jeff King
2020-05-04 21:05     ` Junio C Hamano
2020-05-05  0:14       ` Taylor Blau
2020-05-03 10:16   ` git for-each-ref - sorting by multiple keys clime

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200503091157.GA170902@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=clime7@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).