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

From: Jeff King <peff@peff.net>
Date: Feb 11, 2023, 01:39 UTC
Message-ID: <Y+bxye2zOCMAEJhN@coredump.intra.peff.net>
In-Reply-To: <20230209163400.xi7j3ivd4k6gqoc7@jcai-ubuntu>

On Thu, Feb 09, 2023 at 11:34:00AM -0500, John Cai wrote:

Show 17 quoted lines

> > From the user's perspective, this is weirdly inconsistent with the
> > existing diff attributes, which would be more like:
> > 
> >   # in .gitattributes
> >   *.json diff=json 
> > 
> >   # in config
> >   [diff "json"]
> >   algorithm = histogram
> 
> Thanks for this suggestion, Peff. What I like about this is that it builds off
> of the existing diff.<driver> scheme rather than inventing another one.
> Additionally, we won't get hit with a performance penalty since we already read
> gitattrbitues to see if a driver has been set or not.
> 
> Thinking out loud, if we add "algorithm" as a key for diff.<driver>, it would be
> mutually exclusive with "command" where "command" takes precedence, correct?

Yes. I think the documentation would be something like "When generating a diff internally, use <algorithm> to do so." And external diffs obviously skip that code path internally.

I didn't check whether your patch does this or not, but should this feature (however it is engaged) apply to diff-stats, too? I think --patience, etc, does, which makes sense. But that might be something worth elaborating in the description, too.

-Peff