Re: [PATCHv3 3/7] show: honor --textconv for blobs

[Jeff King <peff@peff.net>]

On Fri, May 10, 2013 at 10:02:51AM -0700, Junio C Hamano wrote:

> > Make "show" on blobs behave like "diff", i.e. honor "--textconv" by
> > default and "--no-textconv" when given.
> [...]
> So "show" on blobs does show the raw contents by default, but the
> user can explicitly ask to enable textconv with --[no-]textconv.  Is
> the second paragraph in the log message still valid?

Yes, I had the same thought...

> > +	if (get_sha1_with_context(obj_name, 0, sha1c, &obj_context))
> > +		die("Not a valid object name %s", obj_name);
> 
> This looks somewhat unfortunate.
> [...]
> I wonder if enriching rev_info->pending with the context information
> might be a clean solution to avoid this redundant but unavoidable
> conversion, but that is a separate and future topic, I think.

It would be, and indeed, that is similar to what the final patch does.
The problem is that it requires an extra allocation (we do not want to
unconditionally put the object_context into the object_array because it
is too big, so we add only a pointer). So having rev_info->pending store
that information would mean that callers would have to know to free it
when freeing the pending array. We would have to either teach each
existing caller to do so, or perhaps enable the behavior only when a
certain flag is set (e.g., rev->keep_object_context or something).

-Peff