On Fri, Oct 21, 2016 at 03:39:27PM -0700, Junio C Hamano wrote: > Now we have identified three callchains that have a set of refs that > they want to show their <old, new> object names in an aligned output, > we can replace their reference to the constant TRANSPORT_SUMMARY_WIDTH > with a helper function call to transport_summary_width() that takes > the set of ref as a parameter. This step does not yet iterate over > the refs and compute, which is left as an exercise to the readers. The final step could be something like this: diff --git a/transport.c b/transport.c index 4dac713063..c1eaa4a 100644 --- a/transport.c +++ b/transport.c @@ -443,7 +443,21 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, int transport_summary_width(const struct ref *refs) { - return (2 * FALLBACK_DEFAULT_ABBREV + 3); + int max_abbrev; + + /* + * Computing the complete set of abbreviated sha1s is expensive just to + * find their lengths, but we can at least find our real dynamic + * minimum by picking an arbitrary sha1. + */ + if (refs) + max_abbrev = strlen(find_unique_abbrev(refs->old_oid.hash, + DEFAULT_ABBREV)); + else + max_abbrev = FALLBACK_DEFAULT_ABBREV; + + /* 2 abbreviated sha1s, plus "..." in between */ + return (2 * max_abbrev + 3); } void transport_print_push_status(const char *dest, struct ref *refs, which produces reasonable results for me. But if we really wanted the true value, I think we'd want to compute and store the abbreviated sha1s, and then the refactoring done by your series probably isn't the right direction. I think we'd instead want to replace "struct strbuf *display" passed down to update_local_ref() with something more like: struct ref_status_table { struct ref_status_item { char old_hash[GIT_SHA1_HEX + 1]; char new_hash[GIT_SHA1_HEX + 1]; char *remote_ref; char *local_ref; char *summary; char code; } *items; size_t alloc, nr; }; and then format_display() would just add to the list (including calling find_unique_abbrev()), and then at the end we'd call a function to show them all. That would also get rid of prepare_format_display(), as we could easily walk over the prepared list before printing any of them (as opposed to what that function does now, which is to walk over the ref map; that requires that it know which refs are going to be printed). -Peff