Re: [PATCH v2] diff: Fix modified lines stats with --stat and --numstat
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 20, 2020, 22:15 UTC
- Message-ID
- <xmqq8sd4gkn3.fsf@gitster.c.googlers.com>
- In-Reply-To
- <xmqqd02ggp84.fsf@gitster.c.googlers.com>
Junio C Hamano <gitster@pobox.com> writes:
Sorry, this will be the last message from me on this topic for now.
> we'd probably need to factor out the condition used in the above > into a helper function, e.g. > > static int cannot_be_the_same(struct diff_filespec *one, struct diff_filespec *two)
The naming of this helper is tricky. In both potential callers, what we want to see is "one and two may be different, we cannot say they are the same with certainty", so "cannot be the same" is a misnomer. Worse, the negated form is hard to grok.
Perhaps "may_differ()" is a more correct name. If either side is NULL oid, we cannot say they are the same, so it is true. If two oid that are not NULL oid are the same, there is no possibility that they are different, so we return false. And two oid that are not NULL oid are different, we know they are different, so we return true.
Show 13 quoted lines
> {
> if ((oideq(&one->oid, &null_oid) || oideq(&two->oid, &null_oid))
> return 1;
> else if (oideq(&one->oid, &two->oid))
> return 0;
> else
> return 1;
> }
>
> and rewrite the conditional in fill_metainfo() to
>
> if (one && two && cannot_be_the_same(one, two)) {
> ...And this becomes much easier to read and understand, i.e.
if (one && two && may_differ(one, two)) {
... create the index one->oid..two->oid header ...Show 5 quoted lines
> The "second best fix" could then become a single liner: > > same_contents = !cannot_be_the_same(one, two); > > using the helper.
And this becomes
same_contents = !may_differ(one, two);
meaning that "there is no possibility that one and two are different". That allows us to optimize out the invocation of the xdl machinery.