All of lore.kernel.org
 help / color / mirror / Atom feed
From: Toon Claes <toon@iotcl.com>
To: Patrick Steinhardt <ps@pks.im>, git@vger.kernel.org
Cc: Taylor Blau <me@ttaylorr.com>, Jeff King <peff@peff.net>
Subject: Re: [PATCH v2] ref-filter: format iteratively with lexicographic refname sorting
Date: Mon, 21 Oct 2024 13:10:44 +0200	[thread overview]
Message-ID: <87ttd5btjf.fsf@iotcl.com> (raw)
In-Reply-To: <e0daa6a2eac97c2b18a53399b7c124fc8d3d238d.1729141657.git.ps@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> [snip]
>
>  ref-filter.c | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index dd195007ce1..424a9cb50ae 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -3244,10 +3244,31 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
>  	return ret;
>  }
>  
> +struct ref_sorting {
> +	struct ref_sorting *next;
> +	int atom; /* index into used_atom array (internal) */
> +	enum ref_sorting_order sort_flags;
> +};
> +
>  static inline int can_do_iterative_format(struct ref_filter *filter,
>  					  struct ref_sorting *sorting,
>  					  struct ref_format *format)
>  {
> +	/*
> +	 * Reference backends sort patterns lexicographically by refname, so if
> +	 * the sorting options ask for exactly that we are able to do iterative
> +	 * formatting.
> +	 *
> +	 * Note that we do not have to worry about multiple name patterns,
> +	 * either. Those get sorted and deduplicated eventually in
> +	 * `refs_for_each_fullref_in_prefixes()`, so we return names in the
> +	 * correct ordering here, too.
> +	 */
> +	if (sorting && (sorting->next ||
> +			sorting->sort_flags ||
> +			used_atom[sorting->atom].atom_type != ATOM_REFNAME))
> +		return 0;
> +
>  	/*
>  	 * Filtering & formatting results within a single ref iteration
>  	 * callback is not compatible with options that require
> @@ -3258,7 +3279,6 @@ static inline int can_do_iterative_format(struct ref_filter *filter,
>  	 */
>  	return !(filter->reachable_from ||
>  		 filter->unreachable_from ||
> -		 sorting ||

Just a small nit, because we remove `sorting` from this condition, I
suggest to also remove the following comment above it:

	 * - sorting the filtered results


Otherwise no comments from my side.

-- 
Toon

  parent reply	other threads:[~2024-10-21 11:11 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-16  6:00 [PATCH] ref-filter: format iteratively with lexicographic refname sorting Patrick Steinhardt
2024-10-16 22:11 ` Taylor Blau
2024-10-17  2:48   ` Jeff King
2024-10-17  4:28     ` Patrick Steinhardt
2024-10-21 12:36       ` karthik nayak
2024-10-21 20:45         ` Taylor Blau
2024-10-17  5:09 ` [PATCH v2] " Patrick Steinhardt
2024-10-17 20:57   ` Taylor Blau
2024-10-21 11:10   ` Toon Claes [this message]
2024-10-21 11:33     ` Patrick Steinhardt
2024-10-21 11:33 ` [PATCH v3] " Patrick Steinhardt
2024-10-21 20:46   ` Taylor Blau

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

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

  git send-email \
    --in-reply-to=87ttd5btjf.fsf@iotcl.com \
    --to=toon@iotcl.com \
    --cc=git@vger.kernel.org \
    --cc=me@ttaylorr.com \
    --cc=peff@peff.net \
    --cc=ps@pks.im \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.