Re: [PATCH 4/4] core.abbrev: raise the default abbreviation to 12 hexdigits

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 30, 2016, 18:40 UTC
Message-ID: <xmqqmvip9qo7.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqqeg42au5w.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 22 quoted lines

> From: Junio C Hamano <gitster@pobox.com>
> Date: Thu, 29 Sep 2016 21:19:20 -0700
> Subject: [PATCH] abbrev: adjust to the new world order
>
> The default_abbrev used to be a concrete value usable as the default
> abbreviation length.  The code that sets custom abbreviation length,
> in response to command line argument, often did something like:
>
> 	if (skip_prefix(arg, "--abbrev=", &arg))
> 		abbrev = atoi(arg);
> 	else if (!strcmp("--abbrev", &arg))
> 		abbrev = DEFAULT_ABBREV;
> 	/* make the value sane */
> 	if (abbrev < 0 || 40 < abbrev)
> 		abbrev = ... some sane value ...
>
> The new world order however is that the default_abbrev is a negative
> value that signals find_unique_abbrev() that it needs to dynamically
> find out a good default value.  We shouldn't coerce a negative value
> into a random positive value like the above sample code.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

There is another instance buried deep in an obscure macro. A minimum fix may look like this, but I really hope somebody else finds a better approach. Peff alluded to "when it is still -1 substituting it with a reasonable value like 7" in a separate thread, and we probably would want a way to allow accessing that "reasonable value like 7" without triggering auto sizing logic too early.

With this and the patch in the message I am responding to, your patch from the last night seems to pass all the tests for me.

 transport.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/transport.h b/transport.h
index 6fe3485325..8a96e22bb0 100644
--- a/transport.h
+++ b/transport.h
@@ -142,7 +142,7 @@ struct transport {
 #define TRANSPORT_PUSH_ATOMIC 8192
 #define TRANSPORT_PUSH_OPTIONS 16384
 
-#define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
+#define TRANSPORT_SUMMARY_WIDTH (2 * (DEFAULT_ABBREV < 0 ? 7 : DEFAULT_ABBREV) + 3)
 #define TRANSPORT_SUMMARY(x) (int)(TRANSPORT_SUMMARY_WIDTH + strlen(x) - gettext_width(x)), (x)
 
 /* Returns a transport suitable for the url */