Re: [PATCH] diff: tighten interaction between -w and --exit-code

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 17, 2023, 16:12 UTC
Message-ID: <xmqqy1i9bqcm.fsf@gitster.g>
In-Reply-To: <20230817051059.GA3006160@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 19 quoted lines

> For the diffstat case, I think we could check the mode here, but there
> are other cases (e.g., adding or deleting an empty file). The code right
> below the hunk I quoted seems to try to deal with that (the "cancelling
> the no-op" your comment mentions). I'm not sure if we want something
> like this:
>
> diff --git a/diff.c b/diff.c
> index 38b57b589f..1dbfdaeff0 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -3853,6 +3853,8 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
>  				&& one->mode == two->mode) {
>  				free_diffstat_file(file);
>  				diffstat->nr--;
> +			} else {
> +				o->found_changes = 1;
>  			}
>  		}
>  	}

That is much better. In all cases where the above diffstat->nr-- is not reached and diffstat is kept is where we found changes, so an even simpler solution that fundamentally cannot go wrong would be to see "diffstat->nr" at the end (i.e. "are we going to show diffstat for *any* filepair?"). If it is non-zero, we did find a difference.

Then we do not have to wonder if that else clause is in the right place, or we have to do something similar to the above for cases where DIFF_FILE_VALID() is not true for both sides (i.e. creation or deletion).

Thanks.