Re: [PATCH 08/15] [GSOC] ref-filter: add cat_file_mode in struct ref_format

From: Christian Couder <christian.couder@gmail.com>
Date: Jul 2, 2021, 22:11 UTC
Message-ID: <CAP8UFD2FjY1mY=GS4ZZHiAxTpsqq7+1ywHfCvdRvJrr8Oo9-qg@mail.gmail.com>
In-Reply-To: <CAPig+cQinUsEZqEDb2_zhi37SdYuBCVEznmFbXPJuFB5cFznKg@mail.gmail.com>

On Fri, Jul 2, 2021 at 9:28 PM Eric Sunshine <sunshine@sunshineco.com> wrote:

Show 12 quoted lines

>
> Aside from the potential maintenance burden of the `atom_type >= ...
> && atom_type <= ...` approach, another problem is that it increases
> cognitive load. The reader has to go reference the enum to fully
> understand the cases to which this code applies. On the other hand,
> the way the patch mentions the enumeration items explicitly, it is
> obvious at-a-glance to which cases the code applies. An additional
> downside of the suggestion is that the range specified by `>=` and
> `<=` may cause some readers to think that there is some sort of
> implicit relationship between the items in the range, which doesn't
> seem to be the case. So, I find the way it's done in the patch
> presently easier to comprehend.

I agree that it's less cognitive load, but maybe it could be improved using a separate function like:

static int reject_atom(int cat_file_mode, enum atom_type atom_type)
{
    if (!cat_file_mode)
        return atom_type == ATOM_REST;

    /* cat_file_mode */
    switch (atom_type) {
    case ATOM_FLAG:
    case ATOM_HEAD:
    case ATOM_PUSH:
    case ATOM_REFNAME:
    case ATOM_SYMREF:
    case ATOM_UPSTREAM:
    case ATOM_WORKTREEPATH:
        return 1;
    default:
        return 0;
    }
}