Subject: Re: [PATCH 2/2] log: do not shorten decoration names too early

From: Jeff King <[email-masked]>
Date: Thu, 14 May 2015 02:33:18 -0400

On Wed, May 13, 2015 at 12:40:35PM -0700, Junio C Hamano wrote:

> The DECORATE_SHORT_REFS option given to load_ref_decorations()
> affects the way a copy of the refname is stored for each decorated
> commit, and this forces later steps like current_pointed_by_HEAD()
> to adjust their behaviour based on this initial settings.
> 
> Instead, we can always store the full refname and then shorten them
> when producing the output.
> 
> Signed-off-by: Junio C Hamano <[email-masked]>
> ---
> 
>  * [1/2] is just the earlier "this should fix it" patch, with
>    adjustments to the existing tests.

Nice. After reading the first one, I was wondering why it did not look
like this one. :)

>    I suspect that it may be a good idea to lose the decoration_flags
>    from load_ref_decorations() and instead make that a new parameter
>    to format_decorations().  That way, the caller could decide which
>    ones to use.  It is not unconceivable to extend "log --format=%d"
>    that shows the decoration in the style given by --decorate arg
>    and let the callers specify two additional formats (i.e. decorate
>    always short, decorate always in full), and for that kind of
>    work, this patch will become a prerequisite.

Yeah, agreed.

While we are on the subject of the name_decoration code, I had
considered at one point replacing the use of the decorate.[ch] hash
table with a commit_slab (you can't do it in the general case, because
decorate.[ch] handles arbitrary objects, but the name_decoration code
only does commits). It would in theory be faster, though I don't know if
the time we spend on the hash table is actually measurable (we make a
lot of queries on it, but it doesn't actually get that big in the first
place).

In case you are looking for something to do with your copious free time.
:)

-Peff