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

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> 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?

>> 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).