Re: [PATCH 3/4] for-each-ref --format='%(symref) %(symref:short)'

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 14, 2010, 10:32 UTC
Message-ID: <7vy6iwp0oy.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20100214063243.GA20630@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> It feels ugly that we need to re-resolve the ref when we simply threw
> that information away earlier from when we called resolve_ref in
> get_ref_dir...

It is true and I indeed thought about it while doing the patch, but "how often would we see symref in the entire refs list, and what percentage of callers would benefit from seeing this information?" stopped me from going further in that direction.

> ...
> If we were going to do this for real, I would be tempted to actually
> change the signature of each_ref_fn to take a struct so this doesn't
> come up again.

That would actually be sensible. Adding symref parameter all over just so that we do not have to re-resolve symbolic refs in a few codepaths feels like a "it would have been nicer if things were that way from day one, but how much does that churn really buys us now" kind of change, but carrying the full information around in the internal API is a "right thing to do".