Re: textconv not invoked when viewing merge commit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> writes:

> On Thu, Apr 14, 2011 at 01:06:19PM -0700, Junio C Hamano wrote:
>
>> 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.
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]