ref-filter: apply --ignore-case to all sorting keys
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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
			
			
				maint
			
			
		
							parent
							
								
									041bc65923
								
							
						
					
					
						commit
						76f9e569ad
					
				|  | @ -711,7 +711,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); | ||||
|  |  | |||
|  | @ -71,7 +71,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; | ||||
|  |  | |||
|  | @ -465,7 +465,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; | ||||
|  |  | |||
|  | @ -2163,6 +2163,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); | ||||
|  |  | |||
|  | @ -109,6 +109,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, | ||||
|  |  | |||
|  | @ -828,4 +828,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 | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue
	
	 Jeff King
						Jeff King