On Tue, Jul 08, 2025 at 03:47:48PM +0200, Karthik Nayak wrote:
> diff --git a/refs.h b/refs.h
> index 7c21aaef3d..7852ad36f3 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -1299,21 +1299,32 @@ struct ref_iterator *refs_ref_iterator_begin(
> */
> int ref_iterator_advance(struct ref_iterator *ref_iterator);
>
> +enum ref_iterator_seek_flag {
> + /*
> + * Also set the seek pattern as a prefix for iteration. This ensures
> + * that only references which match the prefix are yielded.
> + */
> + REF_ITERATOR_SEEK_SET_PREFIX = (1 << 0),
> +};
> +
Nit: I think it's a tiny bit confusing that the documentation of this
enum is split up across here and the doc of `ref_iterator_seek()`. I
think it would be sensible to move the last paragraph of the function
over here so that the whole behaviour of the enum is explained in a
single place.
> /*
> - * Seek the iterator to the first reference with the given prefix.
> - * The prefix is matched as a literal string, without regard for path
> - * separators. If prefix is NULL or the empty string, seek the iterator to the
> + * Seek the iterator to the first reference matching the given seek string.
> + * The seek string is matched as a literal string, without regard for path
> + * separators. If seek is NULL or the empty string, seek the iterator to the
> * first reference again.
> *
> - * This function is expected to behave as if a new ref iterator with the same
> - * prefix had been created, but allows reuse of iterators and thus may allow
> - * the backend to optimize. Parameters other than the prefix that have been
> - * passed when creating the iterator will remain unchanged.
> + * This function is expected to behave as if a new ref iterator has been
> + * created, but allows reuse of existing iterators for optimization.
> + *
> + * When the REF_ITERATOR_SEEK_SET_PREFIX flag is set, the iterator's prefix is
> + * updated to match the seek string, affecting all subsequent iterations. If
> + * not, the iterator seeks to the specified reference and clears any previously
> + * set prefix.
> *
> * Returns 0 on success, a negative error code otherwise.
> */
> -int ref_iterator_seek(struct ref_iterator *ref_iterator,
> - const char *prefix);
> +int ref_iterator_seek(struct ref_iterator *ref_iterator, const char *seek,
> + unsigned int flags);
Another tiny nit: instead of calling the variable `seek` we can just
call it `refname`. That might give a bit more of a hint what you're
actually seeking for.
But other than that I'm happy with the new behaviour, where we are now
consistently either setting or resetting the prefix depending on whether
or not the caller set the flag.
> diff --git a/refs/iterator.c b/refs/iterator.c
> index 766d96e795..f2364bd6e7 100644
> --- a/refs/iterator.c
> +++ b/refs/iterator.c
> @@ -407,13 +408,16 @@ static int prefix_ref_iterator_advance(struct ref_iterator *ref_iterator)
> }
>
> static int prefix_ref_iterator_seek(struct ref_iterator *ref_iterator,
> - const char *prefix)
> + const char *seek, unsigned int flags)
> {
> struct prefix_ref_iterator *iter =
> (struct prefix_ref_iterator *)ref_iterator;
> - free(iter->prefix);
> - iter->prefix = xstrdup_or_null(prefix);
> - return ref_iterator_seek(iter->iter0, prefix);
> +
> + if (flags & REF_ITERATOR_SEEK_SET_PREFIX) {
> + free(iter->prefix);
> + iter->prefix = xstrdup_or_null(seek);
> + }
> + return ref_iterator_se
