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

From: Jeff King <peff@peff.net>
Date: Feb 6, 2013, 22:12 UTC
Message-ID: <20130206221240.GC27507@sigill.intra.peff.net>
In-Reply-To: <7vmwvhmli7.fsf@alter.siamese.dyndns.org>

On Wed, Feb 06, 2013 at 08:53:52AM -0800, Junio C Hamano wrote:

Show 19 quoted lines

> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
> > 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
> > default and "--no-textconv" when given.
> 
> What does "log -p" do currently, and what should it do?  Does/should
> it also use --textconv?
> 
> The --textconv is a natural extension of what --ext-diff provides us,
> so I think it should trigger the same way as how --ext-diff triggers.
> 
> We apply "--ext-diff" for "diff" by default but not for "log -p" and
> "show"; I suspect this may have been for a good reason but I do not
> recall the discussion that led to the current behaviour offhand.

I think Michael's commit message explains the situation badly. --textconv is already on for "git show" (and for "git log") by default. Diffs already use it.

This is more about the fact that when showing a single blob, we do not bother to remember the context of the sha1 lookup, including its pathname. Therefore we were not previously able to apply any .gitattributes to the output. So this patch really does two things:

  1. Pass the information along to show_blob_object so that it can
     look up .gitattributes.

  2. Apply the textconv attribute (if ALLOW_TEXTCONV is on, of course).

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?

-Peff