From: Thomas Guyot Date: Thu, 24 Sep 2020 04:49:05 GMT Subject: Re: [PATCH 1/2] diff: Fix modified lines stats with --stat and --numstat Message-ID: <1d0a60c3-d15e-bcbb-f042-2f8ae06f0de1@gmail.com> In-Reply-To: Hi, On 2020-09-23 16:44, Johannes Schindelin wrote: > Hi Junio, > > On Wed, 23 Sep 2020, Junio C Hamano wrote: > >> Johannes Schindelin writes: >> >>> I believe that that is exactly the reason why we want this: >>> >>> - same_contents = oideq(&one->oid, &two->oid); >>> + same_contents = one->oid_valid && two->oid_valid ? >>> oideq(&one->oid, &two->oid) : !strcmp(one->data, two->data); >> >> Not quite. The other side should either be >> >> one->size == two->size && !memcmp(...) Thanks for all the feedback, that was enlightening. Although I have been silent the past few days I watched this thread with interest. So as Junio pointed out this is merely an optimization - the range-diff test that I corrected also showed two 0-line diffs and I realized there's a block further down that should explicitly removes them, under else if (!same_contents) { We can even remove same_contents entirely and everything work just fine after adjusting the range-diff test - the logic is correct and underlying functions already DTRT. My next patch simplifies the test down to: same_contents = one->oid_valid && two->oid_valid && oideq(&one->oid, &two->oid); My understanding is that oid_valid will never be true for a modified (even mode change) or out of tree file so it's a valid assumption. I'll also rename that variable to "same_oid" - the current name is misleading both ways (true doesn't means there will be diffs, false doesn't mean contents differs). Regards, Thomas