From: Ezekiel Newren <ezekielnewren@gmail.com>
Date: Tue, 23 Sep 2025 01:30:54 GMT
Subject: Re: [PATCH v4 00/12] Cleanup xdfile_t and xrecord_t in xdiff.
Message-ID: <CAH=ZcbAHgCLjpLMzditOg8CW-L1RPohGuQjst=h-rATTSoio3Q@mail.gmail.com>
In-Reply-To: <xmqqecryq8o9.fsf@gitster.g>

On Mon, Sep 22, 2025 at 7:06 PM Junio C Hamano <gitster@pobox.com> wrote:
> > 'changed' is NEVER EVER!!! assigned anything other than 0 or 1 which
> > strictly makes it a bool. It's easy to mistake that because the
> > functions in xprepare.c that deal with NO, YES, and MAYBE are within a
> > few lines of 'changed'. Please re-read xdl_cleanup_records() and
> > xdl_clean_mmatch() very carefully. I will update my commit message to
> > make this more clear.
>
> OK, then there is a variable with some type that is _not_ bool that
> is used in xprepare.c and the code that deal with MAYBE does
> something like
>
>         u8 current_state = MAYBE;
>
>         if (the .changed[line] is NOT valid)
>                 current_state = MAYBE;
>         else if (env->xdf.changed[line])
>                 current_state = YES;
>         else /* false */
>                 current_state = NO;
>
> and then use current_state as a three-way variable, perhaps like
>
>         switch (current_state) {
>         case YES:
>                 do the yes thing;
>                 break;
>         case NO:
>                 do the no thing;
>                 break;
>         case MAYBE:
>                 do the maybe thing;
>                 break;
>         }

I apologize for my previous phrasing. I was not very tactful. Yes, I
think your suggestion is a good idea. I'll incorporate that into my
patches.