Re: textconv not invoked when viewing merge commit

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

quoted

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