Re: [PATCH 2/6] show: obey --textconv for blobs

> On Sat, Apr 20, 2013 at 03:38:53PM +0200, Michael J Gruber wrote:
> 
>>> Wait, this does the opposite of the last patch. If we do want to do
>>> this, shouldn't the last one have been an "expect_failure"?
>>
>> The last patch just documents the status quo, which is not a bug per se.
>> Therefore, no failure, but change in the definition of "success".
> 
> IMHO, the series is easier to review if you it does not go back and
> forth. If you have one patch that says "X is the right behavior", and
> then another patch that flips it to say "Y is the right behavior", the
> reviewer would read each in sequence and want to be convinced by your
> arguments for X and Y. But you probably cannot make a good argument for
> X if you are trying to end up at Y. :)
> 
> So I'd much rather see the test introduced with the desired end
> behavior, marked as expect_failure, and the commit message contain an
> argument about why Y is a good thing (and squashing the tests in with
> the actual fix is often even better, because the fix itself would want
> to contain the same argument).
> 
> Just my two cents as a reviewer.
> 
>> My reasoning is twofold:
>>
>> - consistency between "git show commit" and "git show blob"
> 
> I'm not sure I agree with this line of reasoning. "git show commit" is
> showing a diff, not the file contents; textconv has always been about
> munging the contents to produce a textual diff. It may be reasonable to
> extend its definition to "this is the preferred human view of this
> content, and that happens to be what you would want to produce a diff".
> But I do not think it is necessarily inconsistent not to apply it for
> the blob case.
> 
>> - "git show" is a user facing command, and as such should produce output
>> consumable by humans; whereas "git cat-file" is plumbing and should
>> produce raw data unless told otherwise (-p, --textconv).
> 
> That holds if the textconv is the only (or best) human-readable version
> of the file. And maybe that is reasonable. But is it possible that
> somebody uses "textconv" to produce a better diff of some already
> human-readable format? For example, imagine I define a textconv for XML
> files that normalizes the formatting to make diffs less noisy. When I am
> not looking at a diff, what is the best human-readable version? The
> original, or the normalized one? I'm not sure.
> 
> Note that I'm somewhat playing devil's advocate here. For the cases
> where I have used textconv in the real world, I think I would probably
> prefer seeing the converted contents, and I am happy to call it user
> error if I use "git show HEAD:foo.jpg >bar.jpg" accidentally. But I also
> want to make sure we are not regressing somebody else unnecessarily.