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

From: Karthik Nayak <karthik.188@gmail.com>
Date: Apr 30, 2024, 13:11 UTC
Message-ID: <CAOLa=ZQY5rdUqcy0yTB0haduf9EK7KVLeoAYLyaST4fMn6UyFw@mail.gmail.com>
In-Reply-To: <95d7547b2e8c5305e76888f7dc0a41d2b9e2f558.1714479928.git.ps@pks.im>

In the subject: s/distinuish/distinguish

Patrick Steinhardt <ps@pks.im> writes:

Show 23 quoted lines

> 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.