[PATCH 0/5] ref-filter: small cleanups and fixes

[PATCH 0/5] ref-filter: small cleanups and fixes

[Karthik Nayak]

This series contains a few of the small fixes and comments which I've
gathered from reviews of my earlier series [1] to add the
'--start-after' flag to 'git-for-each-ref(1)'.

Individually each patch doesn't hold too much weight on its own, but
together these small improvements add up. That said, if these patches
are too small for the noise generated, we could simply drop it or
combine some commits together.

This is based on top of 3f2a94875d (The twelfth batch, 2025-07-21) with
'kn/for-each-ref-skip' merged in.

[1]: https://lore.kernel.org/r/20250701-306-git-for-each-ref-pagination-v1-0-4f0ae7c0688f@gmail.com

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 Documentation/git-for-each-ref.adoc |  9 +++++----
 builtin/for-each-ref.c              |  2 +-
 ref-filter.c                        |  5 +++--
 refs/ref-cache.c                    |  5 +++--
 t/t6302-for-each-ref-filter.sh      | 19 +++++++++++++++++++
 5 files changed, 31 insertions(+), 9 deletions(-)

Karthik Nayak (5):
      ref-cache: use 'size_t' instead of int for length
      for-each-ref: fix documentation argument ordering
      for-each-ref: reword the documentation for '--start-after'
      t6302: add test combining '--start-after' with '--exclude'
      ref-filter: use REF_ITERATOR_SEEK_SET_PREFIX instead of '1'



base-commit: 15fcbd8f16a2c119a5319b0657e52fe0f387df46
change-id: 20250721-kn-small-cleanups-a01fe5d2756d

Thanks
- Karthik

[PATCH 1/5] ref-cache: use 'size_t' instead of int for length

[Karthik Nayak]

The commit 090eb5336c (refs: selectively set prefix in the seek
functions, 2025-07-15) modified the ref-cache iterator to support
seeking to a specified marker without setting the prefix.

The commit adds and uses an integer 'len' to capture the length of the
seek marker to compare with the entries of a given directory. Since the
type of the variable is 'int', this is met with a typecast of converting
a `strlen` to 'int' so it can be assigned to the 'len' variable.

This is whole operation is a bit wrong:
1. Since the 'len' variable is eventually used in a 'strncmp', it should
have been of type 'size_t'.
2. This also truncates the value provided from 'strlen' to an int, which
could cause a large refname to produce a negative number.

Let's do the correct thing here and simply use 'size_t' for `len`.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 refs/ref-cache.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/refs/ref-cache.c b/refs/ref-cache.c
index 1d95b56d40..8df7ae43e5 100644
--- a/refs/ref-cache.c
+++ b/refs/ref-cache.c
@@ -498,13 +498,14 @@ static int cache_ref_iterator_seek(struct ref_iterator *ref_iterator,
 		 * indexing to each level as needed.
 		 */
 		do {
-			int len, idx;
+			int idx;
+			size_t len;
 			int cmp = 0;
 
 			sort_ref_dir(dir);
 
 			slash = strchr(slash, '/');
-			len = slash ? slash - refname : (int)strlen(refname);
+			len = slash ? (size_t)(slash - refname) : strlen(refname);
 
 			for (idx = 0; idx < dir->nr; idx++) {
 				cmp = strncmp(refname, dir->entries[idx]->name, len);

-- 
2.49.0

Re: [PATCH 1/5] ref-cache: use 'size_t' instead of int for length

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> The commit 090eb5336c (refs: selectively set prefix in the seek
> functions, 2025-07-15) modified the ref-cache iterator to support
> seeking to a specified marker without setting the prefix.
>
> The commit adds and uses an integer 'len' to capture the length of the
> seek marker to compare with the entries of a given directory. Since the
> type of the variable is 'int', this is met with a typecast of converting
> a `strlen` to 'int' so it can be assigned to the 'len' variable.
>
> This is whole operation is a bit wrong:
> 1. Since the 'len' variable is eventually used in a 'strncmp', it should
> have been of type 'size_t'.
> 2. This also truncates the value provided from 'strlen' to an int, which
> could cause a large refname to produce a negative number.
>
> Let's do the correct thing here and simply use 'size_t' for `len`.
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  refs/ref-cache.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/refs/ref-cache.c b/refs/ref-cache.c
> index 1d95b56d40..8df7ae43e5 100644
> --- a/refs/ref-cache.c
> +++ b/refs/ref-cache.c
> @@ -498,13 +498,14 @@ static int cache_ref_iterator_seek(struct ref_iterator *ref_iterator,
>  		 * indexing to each level as needed.
>  		 */
>  		do {
> -			int len, idx;
> +			int idx;
> +			size_t len;
>  			int cmp = 0;
>  
>  			sort_ref_dir(dir);
>  
>  			slash = strchr(slash, '/');
> -			len = slash ? slash - refname : (int)strlen(refname);
> +			len = slash ? (size_t)(slash - refname) : strlen(refname);

The "strlen()" side is good, but was there recently a discussion on
how to safely convert (slash - refname) that is ptrdiff_t to size_t?

My archive search found a rather old ptrdiff_to_size() proposal
https://lore.kernel.org/git/20241227213729.GA796141@coredump.intra.peff.net/
but I thought there were another discussion thread about casting to size_t
recently.

I _think_ a vanilla cast is safe here, as slash sits always right to
refname (if not NULL, that is), and the difference should fit within
size_t (because the difference is smaller than the size of the
memory block pointed at by slash).

So in short, this looks good.  Will queue.

[PATCH 2/5] for-each-ref: fix documentation argument ordering

[Karthik Nayak]

Improve the 'git-for-each-ref(1)' documentation with two corrections:

1. Add parentheses around `--exclude=<pattern>` to indicate this option
   can be repeated as a complete unit.

2. Move `--stdin | <pattern> ...` to the end, after all flags, since
   `<pattern>` is a positional argument that should appear last in the
   argument list.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 Documentation/git-for-each-ref.adoc | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-for-each-ref.adoc b/Documentation/git-for-each-ref.adoc
index ae61ba642a..a170de4209 100644
--- a/Documentation/git-for-each-ref.adoc
+++ b/Documentation/git-for-each-ref.adoc
@@ -10,11 +10,11 @@ SYNOPSIS
 [verse]
 'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
 		   [(--sort=<key>)...] [--format=<format>]
-		   [--include-root-refs] [ --stdin | <pattern>... ]
-		   [--points-at=<object>]
+		   [--include-root-refs] [--points-at=<object>]
 		   [--merged[=<object>]] [--no-merged[=<object>]]
 		   [--contains[=<object>]] [--no-contains[=<object>]]
-		   [--exclude=<pattern> ...] [--start-after=<marker>]
+		   [(--exclude=<pattern>)...] [--start-after=<marker>]
+           [ --stdin | <pattern>... ]
 
 DESCRIPTION
 -----------

-- 
2.49.0

Re: [PATCH 2/5] for-each-ref: fix documentation argument ordering

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> Improve the 'git-for-each-ref(1)' documentation with two corrections:
>
> 1. Add parentheses around `--exclude=<pattern>` to indicate this option
>    can be repeated as a complete unit.
>
> 2. Move `--stdin | <pattern> ...` to the end, after all flags, since
>    `<pattern>` is a positional argument that should appear last in the
>    argument list.
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  Documentation/git-for-each-ref.adoc | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-for-each-ref.adoc b/Documentation/git-for-each-ref.adoc
> index ae61ba642a..a170de4209 100644
> --- a/Documentation/git-for-each-ref.adoc
> +++ b/Documentation/git-for-each-ref.adoc
> @@ -10,11 +10,11 @@ SYNOPSIS
>  [verse]
>  'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
>  		   [(--sort=<key>)...] [--format=<format>]
> -		   [--include-root-refs] [ --stdin | <pattern>... ]
> -		   [--points-at=<object>]
> +		   [--include-root-refs] [--points-at=<object>]
>  		   [--merged[=<object>]] [--no-merged[=<object>]]
>  		   [--contains[=<object>]] [--no-contains[=<object>]]
> -		   [--exclude=<pattern> ...] [--start-after=<marker>]
> +		   [(--exclude=<pattern>)...] [--start-after=<marker>]
> +           [ --stdin | <pattern>... ]

Shouldn't the last line align with the others?

Re: [PATCH 2/5] for-each-ref: fix documentation argument ordering

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1592 bytes --]

Junio C Hamano <gitster@pobox.com> writes:

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> Improve the 'git-for-each-ref(1)' documentation with two corrections:
>>
>> 1. Add parentheses around `--exclude=<pattern>` to indicate this option
>>    can be repeated as a complete unit.
>>
>> 2. Move `--stdin | <pattern> ...` to the end, after all flags, since
>>    `<pattern>` is a positional argument that should appear last in the
>>    argument list.
>>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>>  Documentation/git-for-each-ref.adoc | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/git-for-each-ref.adoc b/Documentation/git-for-each-ref.adoc
>> index ae61ba642a..a170de4209 100644
>> --- a/Documentation/git-for-each-ref.adoc
>> +++ b/Documentation/git-for-each-ref.adoc
>> @@ -10,11 +10,11 @@ SYNOPSIS
>>  [verse]
>>  'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
>>  		   [(--sort=<key>)...] [--format=<format>]
>> -		   [--include-root-refs] [ --stdin | <pattern>... ]
>> -		   [--points-at=<object>]
>> +		   [--include-root-refs] [--points-at=<object>]
>>  		   [--merged[=<object>]] [--no-merged[=<object>]]
>>  		   [--contains[=<object>]] [--no-contains[=<object>]]
>> -		   [--exclude=<pattern> ...] [--start-after=<marker>]
>> +		   [(--exclude=<pattern>)...] [--start-after=<marker>]
>> +           [ --stdin | <pattern>... ]
>
> Shouldn't the last line align with the others?

Indeed, I blotched it up somehow. Will add it locally and wait a ~day
before sending in a new version.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH 2/5] for-each-ref: fix documentation argument ordering

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> Indeed, I blotched it up somehow. Will add it locally and wait a ~day
> before sending in a new version.

Hopefully no need, as I've tweaked a bit locally while queuing.

Re: [PATCH 2/5] for-each-ref: fix documentation argument ordering

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 384 bytes --]

Junio C Hamano <gitster@pobox.com> writes:

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> Indeed, I blotched it up somehow. Will add it locally and wait a ~day
>> before sending in a new version.
>
> Hopefully no need, as I've tweaked a bit locally while queuing.

Thanks, but since Patrick also pointed out about the 'synopsis' block,
I'll send in a new version with the fix.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH 2/5] for-each-ref: fix documentation argument ordering

[Patrick Steinhardt]

On Thu, Jul 24, 2025 at 10:14:43AM +0200, Karthik Nayak wrote:
> diff --git a/Documentation/git-for-each-ref.adoc b/Documentation/git-for-each-ref.adoc
> index ae61ba642a..a170de4209 100644
> --- a/Documentation/git-for-each-ref.adoc
> +++ b/Documentation/git-for-each-ref.adoc
> @@ -10,11 +10,11 @@ SYNOPSIS
>  [verse]
>  'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
>  		   [(--sort=<key>)...] [--format=<format>]
> -		   [--include-root-refs] [ --stdin | <pattern>... ]
> -		   [--points-at=<object>]
> +		   [--include-root-refs] [--points-at=<object>]
>  		   [--merged[=<object>]] [--no-merged[=<object>]]
>  		   [--contains[=<object>]] [--no-contains[=<object>]]
> -		   [--exclude=<pattern> ...] [--start-after=<marker>]
> +		   [(--exclude=<pattern>)...] [--start-after=<marker>]
> +           [ --stdin | <pattern>... ]

While at it we could also convert this to use `[synopsis]`.

Patrick

Re: [PATCH 2/5] for-each-ref: fix documentation argument ordering

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1027 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> On Thu, Jul 24, 2025 at 10:14:43AM +0200, Karthik Nayak wrote:
>> diff --git a/Documentation/git-for-each-ref.adoc b/Documentation/git-for-each-ref.adoc
>> index ae61ba642a..a170de4209 100644
>> --- a/Documentation/git-for-each-ref.adoc
>> +++ b/Documentation/git-for-each-ref.adoc
>> @@ -10,11 +10,11 @@ SYNOPSIS
>>  [verse]
>>  'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
>>  		   [(--sort=<key>)...] [--format=<format>]
>> -		   [--include-root-refs] [ --stdin | <pattern>... ]
>> -		   [--points-at=<object>]
>> +		   [--include-root-refs] [--points-at=<object>]
>>  		   [--merged[=<object>]] [--no-merged[=<object>]]
>>  		   [--contains[=<object>]] [--no-contains[=<object>]]
>> -		   [--exclude=<pattern> ...] [--start-after=<marker>]
>> +		   [(--exclude=<pattern>)...] [--start-after=<marker>]
>> +           [ --stdin | <pattern>... ]
>
> While at it we could also convert this to use `[synopsis]`.
>
> Patrick

That's a good point, Let me do that.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH 3/5] for-each-ref: reword the documentation for '--start-after'

[Karthik Nayak]

The documentation for '--start-after' states that the flag cannot be
used with general pattern matching. This is a bit vague, since there is
no clear understanding about what 'general' means here. Rewrite the
sentence to be more specific.

While here, fix a typo in the 'OPT_STRING'.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 Documentation/git-for-each-ref.adoc | 3 ++-
 builtin/for-each-ref.c              | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-for-each-ref.adoc b/Documentation/git-for-each-ref.adoc
index a170de4209..48af302a6f 100644
--- a/Documentation/git-for-each-ref.adoc
+++ b/Documentation/git-for-each-ref.adoc
@@ -114,7 +114,8 @@ TAB %(refname)`.
     deleted, modified or added between invocations. Output will only yield those
     references which follow the marker lexicographically. Output begins from the
     first reference that would come after the marker alphabetically. Cannot be
-    used with general pattern matching or custom sort options.
+    used with `--sort=<key>` or `--stdin` options, or the _<pattern>_ argument(s)
+    to limit the refs.
 
 FIELD NAMES
 -----------
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 3f21598046..79a79212c9 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -45,7 +45,7 @@ int cmd_for_each_ref(int argc,
 		OPT_GROUP(""),
 		OPT_INTEGER( 0 , "count", &format.array_opts.max_count, N_("show only <n> matched refs")),
 		OPT_STRING(  0 , "format", &format.format, N_("format"), N_("format to use for the output")),
-		OPT_STRING(  0 , "start-after", &filter.start_after, N_("start-start"), N_("start iteration after the provided marker")),
+		OPT_STRING(  0 , "start-after", &filter.start_after, N_("start-after"), N_("start iteration after the provided marker")),
 		OPT__COLOR(&format.use_color, N_("respect format colors")),
 		OPT_REF_FILTER_EXCLUDE(&filter),
 		OPT_REF_SORT(&sorting_options),

-- 
2.49.0

Re: [PATCH 3/5] for-each-ref: reword the documentation for '--start-after'

[Patrick Steinhardt]

On Thu, Jul 24, 2025 at 10:14:44AM +0200, Karthik Nayak wrote:
> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index 3f21598046..79a79212c9 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c
> @@ -45,7 +45,7 @@ int cmd_for_each_ref(int argc,
>  		OPT_GROUP(""),
>  		OPT_INTEGER( 0 , "count", &format.array_opts.max_count, N_("show only <n> matched refs")),
>  		OPT_STRING(  0 , "format", &format.format, N_("format"), N_("format to use for the output")),
> -		OPT_STRING(  0 , "start-after", &filter.start_after, N_("start-start"), N_("start iteration after the provided marker")),
> +		OPT_STRING(  0 , "start-after", &filter.start_after, N_("start-after"), N_("start iteration after the provided marker")),

Funny how typos like this frequently go through reviews without anybody
noticing :)

Patrick

[PATCH 4/5] t6302: add test combining '--start-after' with '--exclude'

[Karthik Nayak]

The '--start-after' doesn't explicitly mention being compatible with the
'--exclude' flag, generally only incompatibility is explicitly called
out. However, it would be nice to test the compatibility between the
two to avoid future regressions. Let's do that.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 t/t6302-for-each-ref-filter.sh | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index e097db6b02..9b80ea1e3b 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -712,6 +712,25 @@ test_expect_success 'start after, overflow specific reference path' '
 	test_cmp expect actual
 '
 
+test_expect_success 'start after, with exclude pattern' '
+	cat >expect <<-\EOF &&
+	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/spot \
+		--exclude=refs/tags/foo >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'start after, last reference' '
 	cat >expect <<-\EOF &&
 	EOF

-- 
2.49.0

[PATCH 5/5] ref-filter: use REF_ITERATOR_SEEK_SET_PREFIX instead of '1'

[Karthik Nayak]

In the commit 51511d68f4 (for-each-ref: introduce a '--start-after'
option, 2025-07-15), for introducing the '--start-after' flag, the
`ref_iterator_seek()` was modified to also accept a flag. This was to
allow the function to also set the prefix when
'REF_ITERATOR_SEEK_SET_PREFIX' was set.

In `do_filter_refs()` instead of passing the flag, we pass in '1' which
is the value of the flag. While this works, this is definitely hard to
read and introduces inconsistency. Change it to use the flag.

While here, remove the unnecessary 'if (prefix)' clause in the 'else'
statement, since the block already checks for 'prefix'.

Reported-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 ref-filter.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index d5a146de87..4edf0df4cc 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -3254,8 +3254,9 @@ static int do_filter_refs(struct ref_filter *filter, unsigned int type, each_ref
 
 		if (filter->start_after)
 			ret = start_ref_iterator_after(iter, filter->start_after);
-		else if (prefix)
-			ret = ref_iterator_seek(iter, prefix, 1);
+		else
+			ret = ref_iterator_seek(iter, prefix,
+						REF_ITERATOR_SEEK_SET_PREFIX);
 
 		if (!ret)
 			ret = do_for_each_ref_iterator(iter, fn, cb_data);

-- 
2.49.0

Re: [PATCH 0/5] ref-filter: small cleanups and fixes

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> This series contains a few of the small fixes and comments which I've
> gathered from reviews of my earlier series [1] to add the
> '--start-after' flag to 'git-for-each-ref(1)'.
>
> Individually each patch doesn't hold too much weight on its own, but
> together these small improvements add up. That said, if these patches
> are too small for the noise generated, we could simply drop it or
> combine some commits together.
>
> This is based on top of 3f2a94875d (The twelfth batch, 2025-07-21) with
> 'kn/for-each-ref-skip' merged in.
>
> [1]: https://lore.kernel.org/r/20250701-306-git-for-each-ref-pagination-v1-0-4f0ae7c0688f@gmail.com
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  Documentation/git-for-each-ref.adoc |  9 +++++----
>  builtin/for-each-ref.c              |  2 +-
>  ref-filter.c                        |  5 +++--
>  refs/ref-cache.c                    |  5 +++--
>  t/t6302-for-each-ref-filter.sh      | 19 +++++++++++++++++++
>  5 files changed, 31 insertions(+), 9 deletions(-)

Will queue.  All of them made sense.

Thanks.

[PATCH v2 0/5] ref-filter: small cleanups and fixes

[Karthik Nayak]

This series contains a few of the small fixes and comments which I've
gathered from reviews of my earlier series [1] to add the
'--start-after' flag to 'git-for-each-ref(1)'.

Individually each patch doesn't hold too much weight on its own, but
together these small improvements add up. That said, if these patches
are too small for the noise generated, we could simply drop it or
combine some commits together.

This is based on top of 3f2a94875d (The twelfth batch, 2025-07-21) with
'kn/for-each-ref-skip' merged in.

[1]: https://lore.kernel.org/r/20250701-306-git-for-each-ref-pagination-v1-0-4f0ae7c0688f@gmail.com

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
Changes in v2:
- In Patch 2/5, fix a whitespace issue and convert 'git-for-each-ref(1)'
  documentation to use the new synopsis block.
- Link to v1: https://lore.kernel.org/r/20250724-kn-small-cleanups-v1-0-0c70f591de3e@gmail.com

---
 Documentation/git-for-each-ref.adoc | 13 +++++++------
 builtin/for-each-ref.c              |  2 +-
 ref-filter.c                        |  5 +++--
 refs/ref-cache.c                    |  5 +++--
 t/t6302-for-each-ref-filter.sh      | 19 +++++++++++++++++++
 5 files changed, 33 insertions(+), 11 deletions(-)

Karthik Nayak (5):
      ref-cache: use 'size_t' instead of int for length
      for-each-ref: fix documentation argument ordering
      for-each-ref: reword the documentation for '--start-after'
      t6302: add test combining '--start-after' with '--exclude'
      ref-filter: use REF_ITERATOR_SEEK_SET_PREFIX instead of '1'

Range-diff versus v1:

1:  449473efe6 = 1:  cb34545b3a ref-cache: use 'size_t' instead of int for length
2:  b5aec572bc < -:  ---------- for-each-ref: fix documentation argument ordering
-:  ---------- > 2:  3fab0eda31 for-each-ref: fix documentation argument ordering
3:  005a35a4b3 = 3:  49e5aa69f2 for-each-ref: reword the documentation for '--start-after'
4:  162f5de259 = 4:  93babd3e71 t6302: add test combining '--start-after' with '--exclude'
5:  3eb8591385 = 5:  ce90149919 ref-filter: use REF_ITERATOR_SEEK_SET_PREFIX instead of '1'


base-commit: 15fcbd8f16a2c119a5319b0657e52fe0f387df46
change-id: 20250721-kn-small-cleanups-a01fe5d2756d

Thanks
- Karthik

[PATCH v2 1/5] ref-cache: use 'size_t' instead of int for length

[Karthik Nayak]

The commit 090eb5336c (refs: selectively set prefix in the seek
functions, 2025-07-15) modified the ref-cache iterator to support
seeking to a specified marker without setting the prefix.

The commit adds and uses an integer 'len' to capture the length of the
seek marker to compare with the entries of a given directory. Since the
type of the variable is 'int', this is met with a typecast of converting
a `strlen` to 'int' so it can be assigned to the 'len' variable.

This is whole operation is a bit wrong:
1. Since the 'len' variable is eventually used in a 'strncmp', it should
have been of type 'size_t'.
2. This also truncates the value provided from 'strlen' to an int, which
could cause a large refname to produce a negative number.

Let's do the correct thing here and simply use 'size_t' for `len`.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 refs/ref-cache.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/refs/ref-cache.c b/refs/ref-cache.c
index 1d95b56d40..8df7ae43e5 100644
--- a/refs/ref-cache.c
+++ b/refs/ref-cache.c
@@ -498,13 +498,14 @@ static int cache_ref_iterator_seek(struct ref_iterator *ref_iterator,
 		 * indexing to each level as needed.
 		 */
 		do {
-			int len, idx;
+			int idx;
+			size_t len;
 			int cmp = 0;
 
 			sort_ref_dir(dir);
 
 			slash = strchr(slash, '/');
-			len = slash ? slash - refname : (int)strlen(refname);
+			len = slash ? (size_t)(slash - refname) : strlen(refname);
 
 			for (idx = 0; idx < dir->nr; idx++) {
 				cmp = strncmp(refname, dir->entries[idx]->name, len);

-- 
2.50.0

[PATCH v2 2/5] for-each-ref: fix documentation argument ordering

[Karthik Nayak]

Improve the 'git-for-each-ref(1)' documentation with two corrections:

1. Add parentheses around `--exclude=<pattern>` to indicate this option
   can be repeated as a complete unit.

2. Move `--stdin | <pattern> ...` to the end, after all flags, since
   `<pattern>` is a positional argument that should appear last in the
   argument list.

While here, change to using the synopsis block which will automatically
format placeholders in italics and keywords in monospace.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 Documentation/git-for-each-ref.adoc | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-for-each-ref.adoc b/Documentation/git-for-each-ref.adoc
index ae61ba642a..ec3b10e14a 100644
--- a/Documentation/git-for-each-ref.adoc
+++ b/Documentation/git-for-each-ref.adoc
@@ -7,14 +7,14 @@ git-for-each-ref - Output information on each ref
 
 SYNOPSIS
 --------
-[verse]
-'git for-each-ref' [--count=<count>] [--shell|--perl|--python|--tcl]
+[synopsis]
+git for-each-ref [--count=<count>] [--shell|--perl|--python|--tcl]
 		   [(--sort=<key>)...] [--format=<format>]
-		   [--include-root-refs] [ --stdin | <pattern>... ]
-		   [--points-at=<object>]
+		   [--include-root-refs] [--points-at=<object>]
 		   [--merged[=<object>]] [--no-merged[=<object>]]
 		   [--contains[=<object>]] [--no-contains[=<object>]]
-		   [--exclude=<pattern> ...] [--start-after=<marker>]
+		   [(--exclude=<pattern>)...] [--start-after=<marker>]
+		   [ --stdin | <pattern>... ]
 
 DESCRIPTION
 -----------

-- 
2.50.0

[PATCH v2 3/5] for-each-ref: reword the documentation for '--start-after'

[Karthik Nayak]

The documentation for '--start-after' states that the flag cannot be
used with general pattern matching. This is a bit vague, since there is
no clear understanding about what 'general' means here. Rewrite the
sentence to be more specific.

While here, fix a typo in the 'OPT_STRING'.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 Documentation/git-for-each-ref.adoc | 3 ++-
 builtin/for-each-ref.c              | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-for-each-ref.adoc b/Documentation/git-for-each-ref.adoc
index ec3b10e14a..060940904d 100644
--- a/Documentation/git-for-each-ref.adoc
+++ b/Documentation/git-for-each-ref.adoc
@@ -114,7 +114,8 @@ TAB %(refname)`.
     deleted, modified or added between invocations. Output will only yield those
     references which follow the marker lexicographically. Output begins from the
     first reference that would come after the marker alphabetically. Cannot be
-    used with general pattern matching or custom sort options.
+    used with `--sort=<key>` or `--stdin` options, or the _<pattern>_ argument(s)
+    to limit the refs.
 
 FIELD NAMES
 -----------
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 3f21598046..79a79212c9 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -45,7 +45,7 @@ int cmd_for_each_ref(int argc,
 		OPT_GROUP(""),
 		OPT_INTEGER( 0 , "count", &format.array_opts.max_count, N_("show only <n> matched refs")),
 		OPT_STRING(  0 , "format", &format.format, N_("format"), N_("format to use for the output")),
-		OPT_STRING(  0 , "start-after", &filter.start_after, N_("start-start"), N_("start iteration after the provided marker")),
+		OPT_STRING(  0 , "start-after", &filter.start_after, N_("start-after"), N_("start iteration after the provided marker")),
 		OPT__COLOR(&format.use_color, N_("respect format colors")),
 		OPT_REF_FILTER_EXCLUDE(&filter),
 		OPT_REF_SORT(&sorting_options),

-- 
2.50.0

[PATCH v2 4/5] t6302: add test combining '--start-after' with '--exclude'

[Karthik Nayak]

The '--start-after' doesn't explicitly mention being compatible with the
'--exclude' flag, generally only incompatibility is explicitly called
out. However, it would be nice to test the compatibility between the
two to avoid future regressions. Let's do that.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 t/t6302-for-each-ref-filter.sh | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index e097db6b02..9b80ea1e3b 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -712,6 +712,25 @@ test_expect_success 'start after, overflow specific reference path' '
 	test_cmp expect actual
 '
 
+test_expect_success 'start after, with exclude pattern' '
+	cat >expect <<-\EOF &&
+	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/spot \
+		--exclude=refs/tags/foo >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'start after, last reference' '
 	cat >expect <<-\EOF &&
 	EOF

-- 
2.50.0

[PATCH v2 5/5] ref-filter: use REF_ITERATOR_SEEK_SET_PREFIX instead of '1'

[Karthik Nayak]

In the commit 51511d68f4 (for-each-ref: introduce a '--start-after'
option, 2025-07-15), for introducing the '--start-after' flag, the
`ref_iterator_seek()` was modified to also accept a flag. This was to
allow the function to also set the prefix when
'REF_ITERATOR_SEEK_SET_PREFIX' was set.

In `do_filter_refs()` instead of passing the flag, we pass in '1' which
is the value of the flag. While this works, this is definitely hard to
read and introduces inconsistency. Change it to use the flag.

While here, remove the unnecessary 'if (prefix)' clause in the 'else'
statement, since the block already checks for 'prefix'.

Reported-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 ref-filter.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index d5a146de87..4edf0df4cc 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -3254,8 +3254,9 @@ static int do_filter_refs(struct ref_filter *filter, unsigned int type, each_ref
 
 		if (filter->start_after)
 			ret = start_ref_iterator_after(iter, filter->start_after);
-		else if (prefix)
-			ret = ref_iterator_seek(iter, prefix, 1);
+		else
+			ret = ref_iterator_seek(iter, prefix,
+						REF_ITERATOR_SEEK_SET_PREFIX);
 
 		if (!ret)
 			ret = do_for_each_ref_iterator(iter, fn, cb_data);

-- 
2.50.0

Re: [PATCH v2 0/5] ref-filter: small cleanups and fixes

[Patrick Steinhardt]

On Mon, Jul 28, 2025 at 10:20:45PM +0200, Karthik Nayak wrote:
> This series contains a few of the small fixes and comments which I've
> gathered from reviews of my earlier series [1] to add the
> '--start-after' flag to 'git-for-each-ref(1)'.
> 
> Individually each patch doesn't hold too much weight on its own, but
> together these small improvements add up. That said, if these patches
> are too small for the noise generated, we could simply drop it or
> combine some commits together.
> 
> This is based on top of 3f2a94875d (The twelfth batch, 2025-07-21) with
> 'kn/for-each-ref-skip' merged in.
> 
> [1]: https://lore.kernel.org/r/20250701-306-git-for-each-ref-pagination-v1-0-4f0ae7c0688f@gmail.com
> 
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
> Changes in v2:
> - In Patch 2/5, fix a whitespace issue and convert 'git-for-each-ref(1)'
>   documentation to use the new synopsis block.
> - Link to v1: https://lore.kernel.org/r/20250724-kn-small-cleanups-v1-0-0c70f591de3e@gmail.com

Thanks, this version looks good to me.

Patrick