Re: [PATCH 4/4] for-each-ref: introduce a '--skip-until' option

[Patrick Steinhardt <ps@pks.im>]

On Tue, Jul 01, 2025 at 05:03:30PM +0200, Karthik Nayak wrote:
> diff --git a/Documentation/git-for-each-ref.adoc b/Documentation/git-for-each-ref.adoc
> index 5ef89fc0fe..4bf7c66b8c 100644
> --- a/Documentation/git-for-each-ref.adoc
> +++ b/Documentation/git-for-each-ref.adoc
> @@ -14,7 +14,7 @@ SYNOPSIS
>  		   [--points-at=<object>]
>  		   [--merged[=<object>]] [--no-merged[=<object>]]
>  		   [--contains[=<object>]] [--no-contains[=<object>]]
> -		   [--exclude=<pattern> ...]
> +		   [--exclude=<pattern> ...] [--skip-until=<pattern>]
>  
>  DESCRIPTION
>  -----------
> @@ -108,6 +108,10 @@ TAB %(refname)`.
>  --include-root-refs::
>  	List root refs (HEAD and pseudorefs) apart from regular refs.
>  
> +--skip-until::
> +    Skip references up to the specified pattern. Cannot be used with
> +    general pattern matching.
> +
>  FIELD NAMES
>  -----------
>  

Is it "up to and including the specified pattern" or "up to but
excluding the specified pattern"? It would help to make it very explicit
whether the pattern itself would be yielded or not.

> diff --git a/ref-filter.c b/ref-filter.c
> index 7a274633cf..9d0255d5db 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -2714,20 +2716,28 @@ static int for_each_fullref_in_pattern(struct ref_filter *filter,
>  		 * so just return everything and let the caller
>  		 * sort it out.
>  		 */
> -		return refs_for_each_fullref_in(get_main_ref_store(the_repository),
> -						"", NULL, cb, cb_data);
> +		goto non_prefix_iter;
>  	}
>  
>  	if (!filter->name_patterns[0]) {
>  		/* no patterns; we have to look at everything */
> -		return refs_for_each_fullref_in(get_main_ref_store(the_repository),
> -						 "", filter->exclude.v, cb, cb_data);
> +		goto non_prefix_iter;
>  	}
>  
>  	return refs_for_each_fullref_in_prefixes(get_main_ref_store(the_repository),
>  						 NULL, filter->name_patterns,
>  						 filter->exclude.v,
>  						 cb, cb_data);
> +
> +non_prefix_iter:
> +	iter = refs_ref_iterator_begin(get_main_ref_store(the_repository), "",
> +				       NULL, 0, flags);
> +	if (filter->seek)
> +		ret = ref_iterator_seek(iter, filter->seek, 0);

Hm, this interface is somewhat weird now, as we have a split in what the
prefix-string meeks when creating the iterator and seeking it. I think
we should align those two functions.

> +	if (ret)
> +		return ret;
> +
> +	return do_for_each_ref_iterator(iter, cb, cb_data);
>  }
>  
>  /*
> @@ -3200,6 +3210,8 @@ static int do_filter_refs(struct ref_filter *filter, unsigned int type, each_ref
>  	if (!filter->kind)
>  		die("filter_refs: invalid type");
>  	else {

The `if` branch now needs to be updated to have curly braces, as well.

Patrick