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

> On Tue, Oct 07, 2008 at 05:17:08PM +0200, Johannes Sixt wrote:
> 
>>> However, there is at least one conflicting situation: there
>>> is no way to say "use the regular rules to determine whether
>>> this file is binary, but if we do diff it textually, use
>>> this funcname pattern." That is, currently setting diff=foo
>>> indicates that the file is definitely text.
>> I don't get your point. Can you provide an example?
> 
> 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
> 
> Under the old behavior, I have just claimed all of the subdir as text.
> But that's not necessarily what I wanted.

> In practice, this doesn't happen much, because funcname tends to follow
> the file extension, as does binary-ness. So you get "*.java diff=java",
> and you really would be insane to have binary files named *.java. But I
> think it makes the concept clearer to explain, and the code simpler, if
> the various facets of a diff driver are orthogonal. In particular, I
> think this makes things cleaner for adding new driver properties in the
> future.

> As to your complaint (which I think is valid)...
> 
>> 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:
> 
>   - because textconv is for converting binary to text for diffing, the
>     result should always be text. So whenever we do the conversion, we
>     should note that it is no longer binary, and automatically treat the
>     result as a text diff. So in this case, we are explicitly saying
>     that binaryness is _not_ orthogonal to this property of the diff
>     driver.
> 
>   - textconv should arguably just be "canonicalize" or similar. That is,
>     there is no reason you couldn't take something like text XML and
>     canonicalize it for a more readable diff. Or even canonicalize a
>     binary file to get a smaller or more sensible binary diff, if you
>     wanted to.
> 
>     In that case, I think the right thing is to set it back to "unknown,
>     check the file contents" if .binary is not set. So you really are
>     saying "this is just a conversion, treat the canonicalized contents
>     exactly as you would have treated the actual contents, including
>     binary detection magic".