Re: [PATCH v4 4/4] for-each-ref: introduce a '--start-after' option
From: Christian Couder <hidden>
Date: 2025-07-14 16:04:36
On Fri, Jul 11, 2025 at 6:21 PM Karthik Nayak [off-list ref] wrote:
quoted hunk ↗ jump to hunk
/* * 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.
quoted hunk ↗ jump to hunk
/*@@ -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; +
[...]
+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.
+ 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 +'