Re: [PATCH 1/2] diff: Fix modified lines stats with --stat and --numstat

From: Jeff King <peff@peff.net>
Date: Sep 18, 2020, 17:27 UTC
Message-ID: <20200918172747.GD183026@coredump.intra.peff.net>
In-Reply-To: <20200918113256.8699-2-tguyot@gmail.com>

On Fri, Sep 18, 2020 at 07:32:55AM -0400, Thomas Guyot-Sionnest wrote:

Show 7 quoted lines

> In builtin_diffstat(), when both files are coming from "stdin" (which
> could be better described as the file's content being written directly
> into the file object), oideq() compares two null hashes and ignores the
> actual differences for the statistics.
> 
> This patch checks if is_stdin flag is set on both sides and compare
> contents directly.

I'm somewhat puzzled how we could have two filespecs that came from stdin, since we'd generally read to EOF. But looking at the test, it seems this is a weird range-diff hack to set is_stdin.

Looking at your patch:

Show 13 quoted lines

> diff --git a/diff.c b/diff.c
> index a5114fa864..2995527896 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -3681,7 +3681,10 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
>  		return;
>  	}
>  
> -	same_contents = oideq(&one->oid, &two->oid);
> +	if (one->is_stdin && two->is_stdin)
> +		same_contents = !strcmp(one->data, two->data);
> +	else
> +		same_contents = oideq(&one->oid, &two->oid);

...should this actually be checking the oid_valid flag in each filespec? That would presumably cover the is_stdin case, too. I also wonder whether range-diff ought to be using that flag instead of is_stdin.

-Peff