blame: reserve mark column only if necessary

git blame prepends commit hashes of boundary commits with "^", ignored
commits with "?" and unblamable commits with "*" and reserves one column
for them by extending the hash abbreviation, to avoid showing ambiguous
hashes.

This reserved column wastes precious screen space, which can be
especially irritating when using the option -b to blank out boundary
commit hashes and not ignoring any commits.  Reserve it only as needed,
i.e. if any of those cases are actually shown.

Pointed-out-by: Laszlo Ersek <laszlo.ersek@posteo.net>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/git-blame.adoc b/Documentation/git-blame.adoc
index 8808009e87eb1d8f369ef5555ef0cddb5bad4fa5..2b74e455997c8c388af0e24c6a9eb16ff27157d9 100644 (file)
--- a/Documentation/git-blame.adoc
+++ b/Documentation/git-blame.adoc
@@ -88,11 +88,12 @@ include::blame-options.adoc[]
 include::diff-algorithm-option.adoc[]
 
 `--abbrev=<n>`::
-       Instead of using the default _7+1_ hexadecimal digits as the
-       abbreviated object name, use _<m>+1_ digits, where _<m>_ is at
-       least _<n>_ but ensures the commit object names are unique.
-       Note that 1 column
-       is used for a caret to mark the boundary commit.
+       Instead of using the default _7_ hexadecimal digits as the
+       abbreviated object name, use at least _<n>_ digits, but ensure
+       the commit object names are unique.
+       If commits marked with caret (boundary), question mark (ignored)
+       or asterisk (unblamable) are shown, extend unmarked object names
+       to align them.
 
 
 THE DEFAULT FORMAT

diff --git a/builtin/blame.c b/builtin/blame.c
index ffbd3ce5c5a2e39a6140baf7db2129baf38cc4d9..5ae39d0458a813302802eb7e32439d44f02a4fc4 100644 (file)
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -453,6 +453,36 @@ static void determine_line_heat(struct commit_info *ci, const char **dest_color)
        *dest_color = colorfield[i].col;
 }
 
+static inline int maybe_putc(int c, FILE *out)
+{
+       return out ? putc(c, out) : 0;
+}
+
+static size_t print_marks(FILE *out, const struct blame_entry *ent, int opt)
+{
+       size_t len = 0;
+
+       if ((ent->suspect->commit->object.flags & UNINTERESTING) &&
+           !blank_boundary && !(opt & OUTPUT_ANNOTATE_COMPAT)) {
+               maybe_putc('^', out);
+               len++;
+       }
+       if (mark_unblamable_lines && ent->unblamable) {
+               maybe_putc('*', out);
+               len++;
+       }
+       if (mark_ignored_lines && ent->ignored) {
+               maybe_putc('?', out);
+               len++;
+       }
+       return len;
+}
+
+static size_t count_marks(const struct blame_entry *ent, int opt)
+{
+       return print_marks(NULL, ent, opt);
+}
+
 static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent,
                       int opt, struct blame_entry *prev_ent)
 {
@@ -499,23 +529,10 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent,
                if (color)
                        fputs(color, stdout);
 
-               if (suspect->commit->object.flags & UNINTERESTING) {
-                       if (blank_boundary) {
-                               memset(hex, ' ', strlen(hex));
-                       } else if (!(opt & OUTPUT_ANNOTATE_COMPAT)) {
-                               length--;
-                               putchar('^');
-                       }
-               }
-
-               if (mark_unblamable_lines && ent->unblamable) {
-                       length--;
-                       putchar('*');
-               }
-               if (mark_ignored_lines && ent->ignored) {
-                       length--;
-                       putchar('?');
-               }
+               if ((suspect->commit->object.flags & UNINTERESTING) &&
+                   blank_boundary)
+                       memset(hex, ' ', strlen(hex));
+               length -= print_marks(stdout, ent, opt);
 
                printf("%.*s", (int)(length < GIT_MAX_HEXSZ ? length : GIT_MAX_HEXSZ), hex);
                if (opt & OUTPUT_ANNOTATE_COMPAT) {
@@ -647,11 +664,15 @@ static void find_alignment(struct blame_scoreboard *sb, int *option)
        struct blame_entry *e;
        int compute_auto_abbrev = (abbrev < 0);
        int auto_abbrev = DEFAULT_ABBREV;
+       size_t max_marks_count = 0;
 
        for (e = sb->ent; e; e = e->next) {
                struct blame_origin *suspect = e->suspect;
                int num;
+               size_t marks_count = count_marks(e, *option);
 
+               if (max_marks_count < marks_count)
+                       max_marks_count = marks_count;
                if (compute_auto_abbrev)
                        auto_abbrev = update_auto_abbrev(auto_abbrev, suspect);
                if (strcmp(suspect->path, sb->path))
@@ -685,8 +706,12 @@ static void find_alignment(struct blame_scoreboard *sb, int *option)
        max_score_digits = decimal_width(largest_score);
 
        if (compute_auto_abbrev)
-               /* one more abbrev length is needed for the boundary commit */
-               abbrev = auto_abbrev + 1;
+               abbrev = auto_abbrev;
+       if (abbrev < (int)the_hash_algo->hexsz) {
+               abbrev += max_marks_count;
+               if (abbrev > (int)the_hash_algo->hexsz)
+                       abbrev = the_hash_algo->hexsz;
+       }
 }
 
 static void sanity_check_on_fail(struct blame_scoreboard *sb, int baa)
@@ -1047,10 +1072,7 @@ parse_done:
        } else if (show_progress < 0)
                show_progress = isatty(2);
 
-       if (0 < abbrev && abbrev < (int)the_hash_algo->hexsz)
-               /* one more abbrev length is needed for the boundary commit */
-               abbrev++;
-       else if (!abbrev)
+       if (!abbrev)
                abbrev = the_hash_algo->hexsz;
 
        if (revs_file && read_ancestry(revs_file))

diff --git a/t/t8002-blame.sh b/t/t8002-blame.sh
index 7822947f028ee6a78de01a70ab4e1b43716cb0e7..bf04b8273efd56780a89648d9520dfa7ab5d5471 100755 (executable)
--- a/t/t8002-blame.sh
+++ b/t/t8002-blame.sh
@@ -113,8 +113,7 @@ test_expect_success 'set up abbrev tests' '
 '
 
 test_expect_success 'blame --abbrev=<n> works' '
-       # non-boundary commits get +1 for alignment
-       check_abbrev 31 --abbrev=30 HEAD &&
+       check_abbrev 30 --abbrev=30 HEAD &&
        check_abbrev 30 --abbrev=30 ^HEAD
 '
 
@@ -141,10 +140,8 @@ test_expect_success 'blame --abbrev gets truncated with boundary commit' '
 '
 
 test_expect_success 'blame --abbrev -b truncates the blank boundary' '
-       # Note that `--abbrev=` always gets incremented by 1, which is why we
-       # expect 11 leading spaces and not 10.
        cat >expect <<-EOF &&
-       $(printf "%11s" "") (<author@example.com> 2005-04-07 15:45:13 -0700 1) abbrev
+       $(printf "%10s" "") (<author@example.com> 2005-04-07 15:45:13 -0700 1) abbrev
        EOF
        git blame -b --abbrev=10 ^HEAD -- abbrev.t >actual &&
        test_cmp expect actual