Re: [PATCH 1/2] sha1_name: try to use same abbrev length when core.abbrevguard is specified

Junio C Hamano [off-list ref] writes:

The function is given "len" from the caller to specify the minimum length
of the output the caller expects (i.e. even if 4 hexdigits is enough to
identify the given commit in a small project, the caller can say it wants
to see at least 7 hexdigits).  The loop without your patch finds the
shortest prefix whose length is at least that given length that uniquely
identifies the given object (or the shortest prefix that doesn't identify
any existing object if the given sha1 does not exist in the repository).
And then ensures the returned value is longer by the guard as an extra
safety measure, so that later when the project grows, the disambiguation
we find today has a better chance to survive.
...
What am I missing?

I think what may be desirable is to honor the caller-supplied "len" a bit
better.  If an object is uniquely identifiable with only 4-hexdigit today,
and if the caller gives 7 as len and the guard is set to 3, we return 10
hexdigits with the current code.  We should instead return 7 hexdigits in
such a case, as that is in line with the "use 3 extra to give the
disambiguation we find today a better chance to survive".

Perhaps that was what you wanted to do, but I don't think that is what
your code actually does.

quoted

 sha1_name.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 709ff2e..6bb8942 100644
--- a/sha1_name.c
+++ b/sha1_name.c

@@ -197,6 +197,7 @@ const char *find_unique_abbrev(const unsigned char *sha1, int len)
 {
 	int status, exists;
 	static char hex[41];
+	int extra_len = unique_abbrev_extra_length;
 
 	exists = has_sha1_file(sha1);
 	memcpy(hex, sha1_to_hex(sha1), 40);

@@ -208,12 +209,14 @@ const char *find_unique_abbrev(const unsigned char *sha1, int len)
 		if (exists
 		    ? !status
 		    : status == SHORT_NAME_NOT_FOUND) {
-			int cut_at = len + unique_abbrev_extra_length;
+			int cut_at = len + extra_len;
 			cut_at = (cut_at < 40) ? cut_at : 40;
 			hex[cut_at] = 0;
 			return hex;
 		}
 		len++;
+		if (extra_len > 0)
+			extra_len--;
 	}
 	return hex;
 }