Re: [PATCH v4 0/4] for-each-ref: introduce seeking functionality via '--start-after'

From: Christian Couder <christian.couder@gmail.com>
Date: Jul 14, 2025, 16:34 UTC
Message-ID: <CAP8UFD0anB4dPDf99-w6dyZNNGuOKU-b30_k322D0EsWE0cKPQ@mail.gmail.com>
In-Reply-To: <20250711-306-git-for-each-ref-pagination-v4-0-ed3303ad5b89@gmail.com>

On Fri, Jul 11, 2025 at 6:20 PM Karthik Nayak <karthik.188@gmail.com> wrote:

> Initially I was also planning to cleanup all the `refs_for_each...()`
> functions in 'refs.h' by simply using the iterator, but this bloated the
> series. So I've left that for another day.

I wonder if there is a plan to add the '--start-after' option to `git branch` and `git tag` too?

Show 5 quoted lines

> Karthik Nayak (4):
>       refs: expose `ref_iterator` via 'refs.h'
>       ref-cache: remove unused function 'find_ref_entry()'
>       refs: selectively set prefix in the seek functions
>       for-each-ref: introduce a '--start-after' option

Except for the few small comments I left on the two last patches and one below, this looks good to me.

[ ... ]

> Range-diff versus v3:

[ ... ]

Show 8 quoted lines

>      +  struct cache_ref_iterator *iter =
>      +          (struct cache_ref_iterator *)ref_iterator;
>      +
>      +  if (flags & REF_ITERATOR_SEEK_SET_PREFIX) {
>     -+          return cache_ref_iterator_set_prefix(iter, seek);
>     -+  } else if (seek && *seek) {
>     ++          return cache_ref_iterator_set_prefix(iter, refname);
>     ++  } else if (refname && *refname) {

Nit: the `else` here could be removed, but yeah it might be better to
do that in a preparatory patch.

>      +          struct cache_ref_iterator_level *level;
>     -+          const char *slash = seek;
>     ++          const char *slash = refname;
>      +          struct ref_dir *dir;