On Fri, Oct 21, 2016 at 09:39:45PM -0700, Junio C Hamano wrote:
> And this is the final one.
>
> -- >8 --
> From: Junio C Hamano <gitster@xxxxxxxxx>
> Date: Fri, 21 Oct 2016 21:33:06 -0700
> Subject: [PATCH] transport: compute summary-width dynamically
>
> Now all that is left to do is to actually iterate over the refs
> and measure the display width needed to show their abbreviation.
I think we crossed emails. :) This is obviously correct, if we don't
mind paying the find_unique_abbrev cost twice for each sha1.
> int transport_summary_width(const struct ref *refs)
> {
> - return (2 * FALLBACK_DEFAULT_ABBREV + 3);
> + int maxw;
> +
> + for (maxw = -1; refs; refs = refs->next) {
> + maxw = measure_abbrev(&refs->old_oid, maxw);
> + maxw = measure_abbrev(&refs->new_oid, maxw);
> + }
This is a minor style nit, but I think it's better to avoid mixing
unrelated bits between the initialization, condition, and iteration bits
of a for loop. IOW:
int maxw = -1;
for (; refs; refs = refs->next) {
...
}
makes the for-loop _just_ about iteration, and it is more clear that the
manipulation of "maxw" is a side effect of the loop that is valid after
it finishes.
Though in this particular case, the loop and the whole function are
short enough that I don't care that much. Just raising a philosophical
point. :)
-Peff