Re: [PATCH v2] untracked-cache: support '--untracked-files=all' if configured

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 27, 2022, 19:54 UTC
Message-ID: <xmqqbkys3x21.fsf@gitster.g>
In-Reply-To: <CAPMMpoixi3x1PHrSHJPV1GRBzMpuOQ4meMr-fipXuDvz-96MEA@mail.gmail.com>

Tao Klerks <tao@klerks.biz> writes:

Show 15 quoted lines

>> > +     configured_dir_flags = configured_default_dir_flags(istate);
>> > +     if (dir->flags != configured_dir_flags)
>> > +             return NULL;
>>
>> Hmph.  If this weren't necessary, this function does not need to
>> call configured_default_dir_flags(), and it can lose the
>> configured_dir_flags variable, too.  Which means that
>> new_untracked_cache() function does not need to take the flags word
>> as a caller-supplied parameter.  Instead, it can make a call to
>> configured_dir_flags() and assign the result to uc->dir_flags
>> itself, which would have been much nicer.
>
> I've tightened this up a little with an inline call to
> configured_default_dir_flags(), getting rid of the variable, let's see
> if that makes more sense / is cleaner.

The extra variable does not bother me at all. Leaving the room for the caller to screw up and pass an incorrect configred_dir_flags is what disturbs me.

If this caller needs to call configred_default_dir_flags(istate), then we cannot avoid it. And the extra variable needed to call the function only once, store its result, and use that result twice, is perfectly a good thing to have. We don't want to see inline or anything tricky.

Thanks.