Patrick Steinhardt <ps@xxxxxx> writes: > 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; > Right.. > 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. Agreed. Let's keep it as is!
Attachment:
signature.asc
Description: PGP signature
