From: Jeff King <peff@peff.net>
Date: Thu, 04 Sep 2008 03:51:40 GMT
Subject: Re: [PATCH 1/2] add '%d' pretty format specifier to show decoration
Message-ID: <20080904035139.GA28123@sigill.intra.peff.net>
In-Reply-To: <48BF0A5A.2040502@lsrfire.ath.cx>

On Thu, Sep 04, 2008 at 12:06:18AM +0200, René Scharfe wrote:

> The patch below does that by using a static variable to remember if the
> decorations have already been loaded.  That's not too unreasonable, I
> think, because we currently have no way of unloading them.

Yes, I think lazy loading is a good solution here, since there is no
extra context that needs to be known to load the decorations.

> +	case 'd':
> +		if (add_again(sb, &c->decoration))
> +			return 1;
> +		format_decoration(sb, commit);
> +		c->decoration.len = sb->len - c->decoration.off;
> +		return 1;

The add_again chunk code is for things that take a long time (like
finding the shortest abbreviation for some hashes). I'm not sure this
counts as "long", as it is just doing a hash lookup on the decoration.
On the other hand, I doubt the extra couple of bytes used hurts much.

-Peff