Re: [PATCH v2 09/10] ref-filter: properly distinuish pseudo and root refs

Karthik Nayak
2024-04-30T13:11:06Z

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

In the subject: s/distinuish/distinguish

Patrick Steinhardt <ps@pks.im> writes:

> The ref-filter interfaces currently define root refs as either a
> detached HEAD or a pseudo ref. Pseudo refs aren't root refs though, so
> let's properly distinguish those ref types.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> builtin/for-each-ref.c | 2 +-
> ref-filter.c | 16 +++++++++-------
> ref-filter.h | 4 ++--
> refs.c | 18 +-----------------
> refs.h | 18 ++++++++++++++++++
> 5 files changed, 31 insertions(+), 27 deletions(-)
>
> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index 919282e12a..5517a4a1c0 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c
> @@ -98,7 +98,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
> }
>
> if (include_root_refs)
> - flags |= FILTER_REFS_ROOT_REFS;
> + flags |= FILTER_REFS_ROOT_REFS | FILTER_REFS_DETACHED_HEAD;

The only issue I see with this patch is that it makes me think that HEAD
is not a root ref anymore. I get that this is the best way to define the
directives because otherwise you'd need a new flag something like
`FILTER_REFS_ROOT_REFS_WITHOUT_HEAD` and `FILTER_REFS_ROOT_REFS` would
be the summation of that and the HEAD flag.

Apart from this, the patch looks good.

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