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".
And both of those obviously involve changes to patch 4/4.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
