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

[Jeff King <peff@peff.net>]

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

> 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