On Wed, Feb 06, 2013 at 02:23:36PM -0800, Junio C Hamano wrote: > Jeff King <peff@xxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html