Re: [RFC/PATCH 2/4] cat-file: do not die on --textconv without textconv filters

From: Jeff King <peff@peff.net>
Date: Feb 6, 2013, 22:43 UTC
Message-ID: <20130206224353.GG27507@sigill.intra.peff.net>
In-Reply-To: <7vvca59j4n.fsf@alter.siamese.dyndns.org>

On Wed, Feb 06, 2013 at 02:23:36PM -0800, Junio C Hamano wrote:

Show 15 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > Which made me wonder: what happens with:
> >
> >   git cat-file --textconv HEAD
> >
> > It looks like we die just before textconv-ing, because we have no
> > obj_context.path. But that is also unlike all of the other --textconv
> > switches, which mean "turn on textconv if you are showing a blob that
> > supports it" and not "the specific operation is --textconv, apply it to
> > this blob". I don't know if that is worth changing or not.
> 
> OK, so in that sense, "cat-file --textconv HEAD" (or HEAD:) should
> die with "Hey, that is not a blob", in other words, Michael's patch
> does what we want without further tweaks, right?

Right, it will die because we do not find a path in the object_context. For the same reason that "cat-file --textconv $sha1" would die.

A more interesting case is "cat-file --textconv HEAD:Documentation", which does have a path, but not a blob. And I think that speaks to your point that we want to fall-through to the pretty-print case, not the blob case.

> By the way are we sure textconv_object() barfs and dies if fed a non
> blob?  Otherwise the above does not hold, and the semantics become
> "turn on textconv if the object you are showing supports it,
> otherwise it has to be a blob.", I think.

I'm not sure. The sha1 would get passed all the way down to fill_textconv. I think because sha1_valid is set, it will not try to reuse the working tree file, so we will end up in diff_populate_filespec, and we could actually textconv the tree object itself.

So yeah, I think we want a check that makes sure we are working with a blob before we even call that function, and "--textconv" should just be "you must feed me a blob of the form $treeish:$path". In practice nobody wants to do anything else anyway, so let's keep the code paths simple; we can always loosen it later if there is a good reason to do so.

-Peff