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 -- 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