Re: [PATCH v4 4/4] for-each-ref: introduce a '--start-after' option
On Fri, Jul 11, 2025 at 6:21 PM Karthik Nayak <karthik.188@gmail.com> wrote:
Show 61 quoted lines
> /*
> * This is the same as for_each_fullref_in(), but it tries to iterate
> * only over the patterns we'll care about. Note that it _doesn't_ do a full
> @@ -2692,10 +2710,13 @@ static int for_each_fullref_in_pattern(struct ref_filter *filter,
> each_ref_fn cb,
> void *cb_data)
> {
> + struct ref_iterator *iter;
> + int flags = 0, ret = 0;
> +
> if (filter->kind & FILTER_REFS_ROOT_REFS) {
> /* In this case, we want to print all refs including root refs. */
> - return refs_for_each_include_root_refs(get_main_ref_store(the_repository),
> - cb, cb_data);
> + flags |= DO_FOR_EACH_INCLUDE_ROOT_REFS;
> + goto non_prefix_iter;
> }
>
> if (!filter->match_as_path) {
> @@ -2704,8 +2725,7 @@ static int for_each_fullref_in_pattern(struct ref_filter *filter,
> * prefixes like "refs/heads/" etc. are stripped off,
> * so we have to look at everything:
> */
> - return refs_for_each_fullref_in(get_main_ref_store(the_repository),
> - "", NULL, cb, cb_data);
> + goto non_prefix_iter;
> }
>
> if (filter->ignore_case) {
> @@ -2714,20 +2734,29 @@ 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->start_after)
> + ret = start_ref_iterator_after(iter, filter->start_after);
> +
> + if (ret)
> + return ret;
> +
> + return do_for_each_ref_iterator(iter, cb, cb_data);
> }Nit: I wonder if what is under the 'non_prefix_iter' label could be in
a new function and instead of `goto non_prefix_iter` we could return
the result of the new function.
Show 10 quoted lines
> /*
> @@ -3197,9 +3226,11 @@ static int do_filter_refs(struct ref_filter *filter, unsigned int type, each_ref
> init_contains_cache(&filter->internal.no_contains_cache);
>
> /* Simple per-ref filtering */
> - if (!filter->kind)
> + if (!filter->kind) {
> die("filter_refs: invalid type");
> - else {
> + } else {Nit: the `else` could be removed altogether here, but maybe that
should be done in a preparatory patch.
> + const char *prefix = NULL;
> +
Show 16 quoted lines
> +test_expect_success 'start after with specific directory and trailing slash' '
> + cat >expect <<-\EOF &&
> + refs/odd/spot
> + refs/tags/annotated-tag
> + refs/tags/doubly-annotated-tag
> + refs/tags/doubly-signed-tag
> + refs/tags/foo1.10
> + refs/tags/foo1.3
> + refs/tags/foo1.6
> + refs/tags/four
> + refs/tags/one
> + refs/tags/signed-tag
> + refs/tags/three
> + refs/tags/two
> + EOF
> + git for-each-ref --format="%(refname)" --start-after=refs/lost >actual &&
I don't see a trailing slash.
Show 19 quoted lines
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'start after, just behind a specific directory' '
> + cat >expect <<-\EOF &&
> + refs/odd/spot
> + refs/tags/annotated-tag
> + refs/tags/doubly-annotated-tag
> + refs/tags/doubly-signed-tag
> + refs/tags/foo1.10
> + refs/tags/foo1.3
> + refs/tags/foo1.6
> + refs/tags/four
> + refs/tags/one
> + refs/tags/signed-tag
> + refs/tags/three
> + refs/tags/two
> + EOF
> + git for-each-ref --format="%(refname)" --start-after=refs/odd/ >actual &&
Here there is a trailing slash though.
> + test_cmp expect actual
> +'
Christian Couder <christian.couder@gmail.com>