Re: [PATCH v2 01/10] ref-filter.c: really don't sort when using --no-sort

> From: Victoria Dye <vdye@github.com>
>
> When '--no-sort' is passed to 'for-each-ref', 'tag', and 'branch', the
> printed refs are still sorted by ascending refname. Change the handling of
> sort options in these commands so that '--no-sort' to truly disables
> sorting.

> option parsing does not distinguish between "the absence of '--sort'"
> (and/or values for tag.sort & branch.sort) and '--no-sort'. Both result in
> an empty 'sorting_options' string list, which is parsed by
> 'ref_sorting_options()' to create the 'struct ref_sorting *' for the
> command. If the string list is empty, 'ref_sorting_options()' interprets
> that as "the absence of '--sort'" and returns the default ref sorting
> structure (equivalent to "refname" sort).
>
> To handle '--no-sort' properly while preserving the "refname" sort in the
> "absence of --sort'" case, first explicitly add "refname" to the string list
> *before* parsing options. This alone doesn't actually change any behavior,
> since 'compare_refs()' already falls back on comparing refnames if two refs
> are equal w.r.t all other sort keys.
>
> Now that the string list is populated by default, '--no-sort' is the only
> way to empty the 'sorting_options' string list. Update
> 'ref_sorting_options()' to return a NULL 'struct ref_sorting *' if the
> string list is empty, and add a condition to 'ref_array_sort()' to skip the
> sort altogether if the sort structure is NULL. Note that other functions
> using 'struct ref_sorting *' do not need any changes because they already
> ignore NULL values.

> Finally, remove the condition around sorting in 'ls-remote', since it's no
> longer necessary. Unlike 'for-each-ref' et. al., it does *not* do any
> sorting by default. This default is preserved by simply leaving its sort key
> string list empty before parsing options; if no additional sort keys are
> set, 'struct ref_sorting *' is NULL and sorting is skipped.

> diff --git a/ref-filter.c b/ref-filter.c
> index e4d3510e28e..7250089b7c6 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -3142,7 +3142,8 @@ void ref_sorting_set_sort_flags_all(struct ref_sorting *sorting,
>  
>  void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)
>  {
> -	QSORT_S(array->items, array->nr, compare_refs, sorting);
> +	if (sorting)
> +		QSORT_S(array->items, array->nr, compare_refs, sorting);
>  }

> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index 3182abde27f..9918ba05dec 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -1570,9 +1570,10 @@ test_expect_success 'tracking with unexpected .fetch refspec' '
>  
>  test_expect_success 'configured committerdate sort' '
>  	git init -b main sort &&
> +	test_config -C sort branch.sort "committerdate" &&
> +
>  	(
>  		cd sort &&
> -		git config branch.sort committerdate &&
>  		test_commit initial &&
>  		git checkout -b a &&
>  		test_commit a &&
> @@ -1592,9 +1593,10 @@ test_expect_success 'configured committerdate sort' '
>  '
>  
>  test_expect_success 'option override configured sort' '
> +	test_config -C sort branch.sort "committerdate" &&
> +
>  	(
>  		cd sort &&
> -		git config branch.sort committerdate &&
>  		git branch --sort=refname >actual &&
>  		cat >expect <<-\EOF &&
>  		  a

> @@ -1606,10 +1608,70 @@ test_expect_success 'option override configured sort' '
>  	)
>  '
>  
> +test_expect_success '--no-sort cancels config sort keys' '
> +	test_config -C sort branch.sort "-refname" &&
> +
> +	(
> +		cd sort &&
> +
> +		# objecttype is identical for all of them, so sort falls back on
> +		# default (ascending refname)

> +		git branch \
> +			--no-sort \
> +			--sort="objecttype" >actual &&
> +		cat >expect <<-\EOF &&
> +		  a
> +		* b
> +		  c
> +		  main
> +		EOF
> +		test_cmp expect actual
> +	)
> +
> +'
> +
> +test_expect_success '--no-sort cancels command line sort keys' '
> +	(
> +		cd sort &&
> +
> +		# objecttype is identical for all of them, so sort falls back on
> +		# default (ascending refname)
> +		git branch \
> +			--sort="-refname" \
> +			--no-sort \
> +			--sort="objecttype" >actual &&

> +		cat >expect <<-\EOF &&
> +		  a
> +		* b
> +		  c
> +		  main
> +		EOF
> +		test_cmp expect actual
> +	)
> +'
> +
> +test_expect_success '--no-sort without subsequent --sort prints expected branches' '
> +	(
> +		cd sort &&
> +
> +		# Sort the results with `sort` for a consistent comparison
> +		# against expected
> +		git branch --no-sort | sort >actual &&
> +		cat >expect <<-\EOF &&
> +		  a
> +		  c
> +		  main
> +		* b
> +		EOF
> +		test_cmp expect actual
> +	)
> +'
> +
>  test_expect_success 'invalid sort parameter in configuration' '
> +	test_config -C sort branch.sort "v:notvalid" &&
> +
>  	(
>  		cd sort &&
> -		git config branch.sort "v:notvalid" &&
>  
>  		# this works in the "listing" mode, so bad sort key
>  		# is a dying offence.