Hi, On 2020-09-23 16:44, Johannes Schindelin wrote: > Hi Junio, > > On Wed, 23 Sep 2020, Junio C Hamano wrote: > >> Johannes Schindelin <Johannes.Schindelin@xxxxxx> 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