On Tue, Apr 30, 2024 at 06:11:05AM -0700, Karthik Nayak wrote:
> In the subject: s/distinuish/distinguish
>
> Patrick Steinhardt <ps@xxxxxx> 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@xxxxxx>
> > ---
> > 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.
Well, it is a root ref, but we treat it differently in the ref-filter
interface because it's rendered differently than any other root ref.
Furthermore, the ref-filter interfaces allow you to _only_ list the HEAD
ref, which is another reason why it's singled out.
Renaming this to FILTER_REFS_ROOT_REFS_WITHOUT_HEAD ould be quite
misleading, too, because we have the following snippet:
@@ -2794,11 +2796,11 @@ static struct ref_array_item *apply_ref_filter(const char *refname, const struct
/*
* Generally HEAD refs are printed with special description denoting a rebase,
* detached state and so forth. This is useful when only printing the HEAD ref
* But when it is being printed along with other root refs, it makes sense to
* keep the formatting consistent. So we mask the type to act like a root ref.
*/
if (filter->kind & FILTER_REFS_ROOT_REFS && kind == FILTER_REFS_DETACHED_HEAD)
kind = FILTER_REFS_ROOT_REFS;
else if (!(kind & filter->kind))
return NULL;
If we named this FILTER_REFS_ROOT_REFS_WITHOUT_HEAD then the above code
would be even more surprising.
So yeah, it's a bit weird, but I think it's more sensible to retain the
code as proposed.
Patrick
Attachment:
signature.asc
Description: PGP signature
