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

From: Jeff King <peff@peff.net>
Date: May 14, 2015, 06:33 UTC
Message-ID: <20150514063317.GA22509@peff.net>
In-Reply-To: <xmqqoalo9sgc.fsf_-_@gitster.dls.corp.google.com>

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

Show 13 quoted lines

> 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 <gitster@pobox.com>
> ---
> 
>  * [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. :)

Show 8 quoted lines

>    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