Re: [PATCH 2/2] diff: teach diff to read gitattribute diff-algorithm

From: Jeff King <peff@peff.net>
Date: Feb 15, 2023, 15:00 UTC
Message-ID: <Y+zzh80fybq8Tn66@coredump.intra.peff.net>
In-Reply-To: <2ca47f26-f10a-1fae-c879-f9479f7acdb1@dunelm.org.uk>

On Wed, Feb 15, 2023 at 02:44:59PM +0000, Phillip Wood wrote:

Show 12 quoted lines

> Indeed. Ironically as there are no unique context lines in that example the
> blank lines are being matched by patience implementation falling back to the
> myers algorithm. Normally the myers implementation tries to avoid matching
> common context lines between two blocks of changed lines but I think because
> in this case it is only called on a small part of the file the blank lines
> are not common enough to trigger that heuristic. I've got a patch[1] that
> stops the patience implementation falling back to the myers algorithm and
> just trims any leading and trailing context. On the whole it I think it
> gives more readable diffs but I've not got any systematic data to back that
> up. I also suspect there are pathological cases such as each line in the
> file being duplicated where the falling back to the myers algorithm gives a
> much better result.

Ah, I should have suspected it was something like that (since one of the purposes of patience is trying not to key on meaningless lines).

I tried your patch on my test case, and the result is even more readable than the myers output, because it really was effectively a complete rewrite of the function. It is, of course, not the minimal diff. I'm not sure if there would be cases where you'd prefer the minimal. I guess if each stanza's change really was independent of the others. But if there is no commonality except for blank lines, I find it hard to imagine that it's much worse to just treat the whole thing as a block.

Anyway, thank you (and Elijah) for explaining. I'm getting more comfortable with the idea of switching the default.

-Peff