Re: [RFC/PATCH 1/4] show: obey --textconv for blobs

From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Michael J Gruber <git@drmicha.warpmail.net>, git@vger.kernel.org
Subject: Re: [RFC/PATCH 1/4] show: obey --textconv for blobs
Date: Wed, 6 Feb 2013 19:10:13 -0500	[thread overview]
Message-ID: <20130207001013.GA28970@sigill.intra.peff.net> (raw)
In-Reply-To: <7vr4kt9f53.fsf@alter.siamese.dyndns.org>

On Wed, Feb 06, 2013 at 03:49:44PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > And stating it that way makes it clear that there may be other missing
> > steps (3 and up) to apply other gitattributes. For example, should "git
> > show $blob" respect crlf attributes? Smudge filters?
> 
> Yeah, I think applying these when path is availble may make sense.
> 
> Are we going to teach cat-file to honor them with "--textconv" and
> possibly other new options?
> 
> Or should the "--textconv" imply application of these other filters?
> I am tempted to say yes, but I haven't thought things through...

For diff, we should already recognize them (because we feed the diff
machinery the path name, and it does the usual attributes lookup). Of
course it only uses things like funcname, not any of the
convert-to-filesystem options (hmm, actually, I guess it may use
convert-from-filesystem, but in such a case it would not be reading from
a blob anyway, but from a filesystem path, so it is not related to this
code).

For cat-file, I don't think --textconv should necessarily imply it. The
original motivation for "cat-file --textconv" was for external blame to
be able to access the converted contents. But it would not want to do
filesystem-level conversion; it wants the canonical version of the file.
I think a better option name for smudge/crlf would be "--to-filesystem"
or something like that. And probably it should not be the default.

git-show should probably have the same option. I doubt it should be on
by default, but I can see it being useful for:

  git show --to-filesystem HEAD:Makefile >Makefile

or whatever. I don't think those features necessarily need to come as
part of Michael's series. They can wait for people who care to implement
them. But I do think the refactoring of passing along the path from the
object_context should keep in mind that we will probably want to go that
way eventually.

Are there other attributes that we might care about when showing a blob?
The only ones I can think of are the ones for converting to the
filesystem representation.

-Peff