Re: [PATCH 3/4] diff: introduce diff.<driver>.binary

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 12, 2008, 05:24 UTC
Message-ID: <7vabdaidwj.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20081007153543.GA26531@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 9 quoted lines

> Let's say I have a subdirectory of files, some of which are binary. But
> for those that _aren't_ binary, I want to use a particular funcname
> pattern. So I do this:
>
>   echo '* diff=foo' >subdir/.gitattributes
>   git config diff.foo.funcname some-regex
> ...
> In practice, this doesn't happen much, because funcname tends to follow
> the file extension, as does binary-ness.

I find this a highly contrived example. Is this ever be useful in practice?

Show 8 quoted lines

>> The reason why I'd like to understand the issue is because I like the
>> diff.foo.textconv that you introduce in patch 4/4, but I dislike that I
>> have to set diff.foo.binary to false in order to use the textconv. So, why
>> is this .binary needed?
>
> I think this .binary is something we can and should get rid of; as it
> stands now, you always end up having to set it along with .textconv. I
> have considered two ways:

The logic behind the original behaviour was that the file ought to be "diff-able" if you are setting up funcname pattern because the funcname pattern only makes sense if you are doing the textual diff. In other words, "should we do textual diff?" and "what funcname pattern should we use?" are _not_ orthogonal, as wanting to configure the latter does imply that you do want to see the change in the textual diff format.

For the same rationale, if you have .textconv, I think it is natural for us to say that you do want to see the change in the textual diff format. So I'd agree that you can get rid of this .binary business by saying that having .textconv marks it diffable (IOW, I think your first alternative makes more sense).