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

2013-04-23 Thread Michael J Gruber
Matthieu Moy venit, vidit, dixit 22.04.2013 17:54: > Jeremy Rosen writes: > >> some features detect if they are piping to a terminal... couldn't we do >> something like that ? > > That's OK for convenience features like colors or so, but that would be > really, really unexpected to have > > $ g

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

2013-04-22 Thread Matthieu Moy
Jeremy Rosen writes: > some features detect if they are piping to a terminal... couldn't we do > something like that ? That's OK for convenience features like colors or so, but that would be really, really unexpected to have $ git show HEAD:file foo $ git show HEAD:file > tmp $ cat tmp bar IMH

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

2013-04-22 Thread Jeremy Rosen
> > git show --textconv HEAD~4:binary-gob | less > > > > but I doubt it is a good idea to turn it on by default this late in > > the game. > > Exactly. I certainly do not mind it as an option, and I am on the > fence > regarding it as a default (I think it might have been a sane thing to > do

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

2013-04-22 Thread Jeff King
On Mon, Apr 22, 2013 at 08:25:41AM -0700, Junio C Hamano wrote: > True. Applying textconv to otherwise unreadable blobs is often > useful, but I agree that it is unexpected if it is done by default, > especially given that many people have learned to do: > > git show HEAD~4:binary-gob >old

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

2013-04-22 Thread Junio C Hamano
Jeff King writes: > 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

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

2013-04-22 Thread Michael J Gruber
Jeff King venit, vidit, dixit 21.04.2013 05:37: > 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 stat

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

2013-04-20 Thread Jeff King
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 fa

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

2013-04-20 Thread Michael J Gruber
Jeff King venit, vidit, dixit 20.04.2013 06:06: > On Fri, Apr 19, 2013 at 06:44:45PM +0200, Michael J Gruber wrote: > >> Currently, "diff" and "cat-file" for blobs obey "--textconv" options >> (with the former defaulting to "--textconv" and the latter to >> "--no-textconv") whereas "show" does not

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

2013-04-19 Thread Jeff King
On Fri, Apr 19, 2013 at 06:44:45PM +0200, Michael J Gruber wrote: > Currently, "diff" and "cat-file" for blobs obey "--textconv" options > (with the former defaulting to "--textconv" and the latter to > "--no-textconv") whereas "show" does not obey this option, even though > it takes diff options.

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

2013-04-19 Thread Michael J Gruber
Currently, "diff" and "cat-file" for blobs obey "--textconv" options (with the former defaulting to "--textconv" and the latter to "--no-textconv") whereas "show" does not obey this option, even though it takes diff options. Make "show" on blobs behave like "diff", i.e. obey "--textconv" by defaul