Re: textconv not invoked when viewing merge commit

[Jeff King <peff@peff.net>]

On Thu, Apr 14, 2011 at 02:05:07PM -0700, Junio C Hamano wrote:

> > Yeah, I think that is pretty readable. But it gives me a funny feeling
> > to encode magic strings inside actual diff output. That is, the output
> > is indistinguishable from a file which contained the "Binary blob..."
> > strings.
>[...]
> 
> Yeah, that may be a sensible concern.
> 
> If we really cared, I would say that plumbing should keep the current
> behaviour (line-by-line even for binaries, and not using textconv unless
> it is asked).

I disagree. Spewing binary contents in the middle of patch output is
wrong and a bug, and we should fix it. Not to mention that the results
are simply incomprehensible in many cases. Binary data isn't
line-oriented, and treating it that way is just going to produce
confusing and useless results. Not to mention that I wouldn't be
surprised if embedded NULs in the data are not being handled properly by
the diff code.

I would much rather have it say "Binary files differ". It's not that
informative, but at least you don't waste a lot of time trying to figure
out what in the world it means.

> Having said all that, I don't think we made -c/--cc available to plumbing
> on purpose; rather they happen to be available because we thought people
> with common sense wouldn't run things like "diff-tree --c" that are meant
> for human consumption and expect the result to be parsable by their
> scripts. In other words, making the parser barf only for plumbing was not
> worth doing.

Weren't they needed originally for "git rev-list | git diff-tree"? Maybe
they post-date the invention of actual C "git log"; I didn't look. At
any rate, they've been around for a while, and it is not unreasonable
for somebody to want to script around the generation of human-readable
output, so I think they are a good addition.

I think the real argument to be made is that "--cc" was never parseable,
because it can't be applied, and users of the format should know that. I
sort of buy that. Though you could also potentially do other kinds of
analysis on --cc output (e.g., something blame-ish but totally external
to git). And for that you wouldn't want to pretend content was there
that isn't. It's an edge case, certainly, but I don't see any reason not
to be conservative in what we generate. The "Binary files differ" type
of output is not that much harder to generate.

-Peff