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

From: Jeff King <peff@peff.net>
Date: Feb 7, 2013, 00:10 UTC
Message-ID: <20130207001013.GA28970@sigill.intra.peff.net>
In-Reply-To: <7vr4kt9f53.fsf@alter.siamese.dyndns.org>

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

Show 13 quoted lines

> 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