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

From: Junio C Hamano <[email-masked]>
Date: Thu, 17 Aug 2023 09:12:09 -0700

Jeff King <[email-masked]> writes:

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