Re: [PATCH 3/3] transport: allow summary-width to be computed dynamically
On Fri, Oct 21, 2016 at 03:39:27PM -0700, Junio C Hamano wrote:
Show 6 quoted lines
> 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
Jeff King <peff@peff.net>