Re: [PATCH 3/3] transport: allow summary-width to be computed dynamically

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]