Jeff King [off-list ref] writes:
On Thu, Apr 14, 2011 at 01:06:19PM -0700, Junio C Hamano wrote:
quoted
Instead, I think we should just use "Binary blob $SHA-1\n" as if that is
the textconv of a binary file without textconv filter. That would
certainly make the code much simpler, and more importantly, the output
would become more pleasant. We would show something like:
- Binary blob bc3c57058faba66f6a7a947e1e9642f47053b5bb
-Binary blob 536e55524db72bd2acf175208aef4f3dfc148d42
++Binary blob 67cfeb2016b24df1cb406c18145efd399f6a1792
if we did so.
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.
I can't think of a case where it matters, though, so maybe it is just
paranoia.
We do something similar for textconv, of course, but we always knew that
was a human-only thing, and it isn't enabled for plumbing commands. This
would be.
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). If the command line asked for --textconv, we can use that
"Binary blob $SHA-1" string as a fallback textconv result for binary blobs
that do not have any textconv filter configured. So the additional logic
to convert the final image and parent images (two places to patch) would
become more like:
if (if we are a Porcelain or --textconv option given) {
if (path has textconv)
use textconv;
else if (path is binary)
use "Binary blob $SHA-1";
}
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.