Jeff King <peff@xxxxxxxx> writes:
> 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...
I'd rewrite the paragraph to something like:
Make "show" on blobs honor "--textconv" when it is asked. The default
is not to apply textconv, which is in line with what "cat-file" does.
>> > + 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.
OK, I wasn't paying attention ;-)
> 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).
One thing to notice is that those accessing rev->pending before
calling prepare_revision_walk(), as opposed to those receiving
objects in rev->commits via get_revision(), are the only ones that
care about the context and wants to act differently depending on
where these came from and how they were specified.
That suggests at least two possibilities to me:
- Perhaps we can place the context in rev->pending and clear them
when prepare_revision_walk() moves them to rev->commits, without
introducing rev->keep_object_context?
- Perhaps instead of extending object-array, we can move this kind
of information to rev_cmdline and enrich that structure?
--
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