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

From: Junio C Hamano <[email-masked]>
Date: Sun, 27 Feb 2022 11:54:14 -0800

Tao Klerks <[email-masked]> writes:

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