Re: [PATCH 3/5] ref-filter: add a "detached_head_first" sorting option

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 6, 2021, 23:45 UTC
Message-ID: <xmqqy2h5oci9.fsf@gitster.c.googlers.com>
In-Reply-To: <20210106100139.14651-4-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

Show 9 quoted lines

> +static int compare_detached_head(struct ref_array_item *a, struct ref_array_item *b)
> +{
> +	if (a->kind & FILTER_REFS_DETACHED_HEAD)
> +		return -1;
> +	else if (b->kind & FILTER_REFS_DETACHED_HEAD)
> +		return 1;
> +	BUG("compare_detached_head() is guarded by an xor on [ab]->kind & FILTER_REFS_DETACHED_HEAD");
> +	return 0;
> +}

OK.

Show 5 quoted lines

> +void ref_sorting_detached_head_first_all(struct ref_sorting *sorting, int flag)
> +{
> +	for (; sorting; sorting = sorting->next)
> +		sorting->detached_head_first = !!flag;
> +}

This, taken together with existing ref_sorting_icase_all(), looks somewhat ugly, especially when you ponder how you would add a third similar option to the mix.

Perhaps "ignore_case" and "detached_head_first" shouldn't be separate bitfields, but bits in the same flag word member in the "struct ref_sorting", and "set/unset these flags to all the sort ops" helper function should just take a flags word that has two bits?

Or maybe it is good enough for now. I hesitate to say so myself, though, after already saying it is "somewhat ugly" ;-)

Show 24 quoted lines

>  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 feaef4a8fde..3b92e0f2696 100644
> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -30,6 +30,7 @@ struct ref_sorting {
>  	int atom; /* index into used_atom array (internal) */
>  	unsigned reverse : 1,
>  		ignore_case : 1,
> +		detached_head_first : 1,
>  		version : 1;
>  };
>  
> @@ -111,6 +112,8 @@ int verify_ref_format(struct ref_format *format);
>  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);
> +/*  Set the detached_head_first flag for all elements of a sorting list */
> +void ref_sorting_detached_head_first_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,