Re: [PATCH v4 00/12] Cleanup xdfile_t and xrecord_t in xdiff.

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 23, 2025, 01:06 UTC
Message-ID: <xmqqecryq8o9.fsf@gitster.g>
In-Reply-To: <CAH=ZcbC5Y04D4bGjfH3rZ8GKabDttFez5qb9i8mXVsfE3LF26w@mail.gmail.com>

Ezekiel Newren <ezekielnewren@gmail.com> writes:

Show 20 quoted lines

>> How about
>>
>>  - rename rchg[] to changed[], which is a very good move;
>>
>>  - optionally make it unsigned char, not char;
>>
>>  - the user of changed[] that uses only 0 or 1 and is not even aware
>>    of that MAYBE thing use 0 or 1;
>>
>>  - the user of changed[] that has to be aware of that MAYBE state
>>    use its own NO/YES/MAYBE for readability.
>>
>> Hmm?
>
> '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;
	}