diff_unique_abbrev(): document its assumption and limitation

This function is used to add "..." to displayed object names in
"diff --raw --abbrev[=<n>]" output.  It bases its behaviour on an
untold assumption that the abbreviation length requested by the
caller is "reasonble", i.e. most of the objects will abbreviate
within the requested length and the resulting length would never
exceed it by more than a few hexdigits (otherwise the resulting
columns would not align).  Explain that in a comment.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/diff.c b/diff.c
index 7a4309397cb7fd6819d6f643b259c2bc3abe202f..01e1507e74b02753aef74aac4e427a0b0804878b 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -4089,7 +4089,8 @@ void diff_free_filepair(struct diff_filepair *p)
        free(p);
 }
 
        free(p);
 }
 

-/* This is different from find_unique_abbrev() in that
+/*
+ * This is different from find_unique_abbrev() in that

  * it stuffs the result with dots for alignment.
  */
 const char *diff_unique_abbrev(const unsigned char *sha1, int len)
  * it stuffs the result with dots for alignment.
  */
 const char *diff_unique_abbrev(const unsigned char *sha1, int len)


@@ -4101,6 +4102,26 @@ const char *diff_unique_abbrev(const unsigned char *sha1, int len)
 
        abbrev = find_unique_abbrev(sha1, len);
        abblen = strlen(abbrev);
 
        abbrev = find_unique_abbrev(sha1, len);
        abblen = strlen(abbrev);

+
+       /*
+        * In well-behaved cases, where the abbbreviated result is the
+        * same as the requested length, append three dots after the
+        * abbreviation (hence the whole logic is limited to the case
+        * where abblen < 37); when the actual abbreviated result is a
+        * bit longer than the requested length, we reduce the number
+        * of dots so that they match the well-behaved ones.  However,
+        * if the actual abbreviation is longer than the requested
+        * length by more than three, we give up on aligning, and add
+        * three dots anyway, to indicate that the output is not the
+        * full object name.  Yes, this may be suboptimal, but this
+        * appears only in "diff --raw --abbrev" output and it is not
+        * worth the effort to change it now.  Note that this would
+        * likely to work fine when the automatic sizing of default
+        * abbreviation length is used--we would be fed -1 in "len" in
+        * that case, and will end up always appending three-dots, but
+        * the automatic sizing is supposed to give abblen that ensures
+        * uniqueness across all objects (statistically speaking).
+        */

        if (abblen < 37) {
                static char hex[41];
                if (len < abblen && abblen <= len + 2)
        if (abblen < 37) {
                static char hex[41];
                if (len < abblen && abblen <= len + 2)