git-blame vs. abbrev

git-blame vs. abbrev

[Laszlo Ersek]

Hi,

when git-blame is passed the "-b" option ("Show blank SHA-1 for boundary 
commits"), shouldn't git-blame *stop* reserving a commit hash nibble for 
the caret that otherwise marks boundary commits?

More directly, I find it inconvenient that git-blame shows commit hashes 
that are one nibble longer (13) than my "core.abbrev" (12) setting; that 
makes cutting and pasting commit hashes from the git-blame output into a 
git-rebase TODO list cumbersome. I briefly hoped that by setting 
"blame.blankBoundary", I could get around that, but it doesn't seem to 
work (I tried with Git 2.55). I now have an alias that passes 
"--abbrev=11" explicitly, as a last resort, to git-blame. (Even a 
potential "blame.abbrev" would be superior, but such a permanent setting 
doesn't seem to exist.)

Thanks,
Laszlo Ersek

Re: git-blame vs. abbrev

[Junio C Hamano]

Laszlo Ersek <laszlo.ersek@posteo.net> writes:

> Hi,
>
> when git-blame is passed the "-b" option ("Show blank SHA-1 for boundary 
> commits"), shouldn't git-blame *stop* reserving a commit hash nibble for 
> the caret that otherwise marks boundary commits?
>
> More directly, I find it inconvenient that git-blame shows commit hashes 
> that are one nibble longer (13) than my "core.abbrev" (12) setting; that 
> makes cutting and pasting commit hashes from the git-blame output into a 
> git-rebase TODO list cumbersome.

I never knew that the parser in rebase did not want to see a longer
abbreviation; shouldn't it take 16 hexadecimal abbreviation from the
result of letting the user edit the list, even if it initially gave
12 hexadecimal abbreviation, as long as these extra 4 hexdigits do
not break the commit object name?  That is a more serious usability
bug that needs to be fixed, if it is the case, I would think.

FWIW, even if your core.abbrev says you want 12, if two objects
share the same 12 hexdigits as the prefix, you do end up getting 13
or more, so a parser that insists on exact 12 hexdigits sounds like
a bug.

Just for the sake of aesthetics, I agree that when we are not
showing the boundary mark, it would make sense not to reserve one
column that we know we will never use.  But unless there is a
mistaken parser that insists on 12 hexdigits when 13 hexdigits you
give uniquely identify the same object, I suspect you wouldn't even
notice that the hexadecimal digits you see on the screen have one
digit longer than usual ;-).

Re: git-blame vs. abbrev

[Laszlo Ersek]

On 6/30/26 21:49, Junio C Hamano wrote:
> Laszlo Ersek <laszlo.ersek@posteo.net> writes:
>
>> Hi,
>>
>> when git-blame is passed the "-b" option ("Show blank SHA-1 for
>> boundary commits"), shouldn't git-blame *stop* reserving a commit
>> hash nibble for the caret that otherwise marks boundary commits?
>>
>> More directly, I find it inconvenient that git-blame shows commit
>> hashes that are one nibble longer (13) than my "core.abbrev" (12)
>> setting; that makes cutting and pasting commit hashes from the
>> git-blame output into a git-rebase TODO list cumbersome.
>
> I never knew that the parser in rebase did not want to see a longer
> abbreviation; shouldn't it take 16 hexadecimal abbreviation from the
> result of letting the user edit the list, even if it initially gave
> 12 hexadecimal abbreviation, as long as these extra 4 hexdigits do
> not break the commit object name?  That is a more serious usability
> bug that needs to be fixed, if it is the case, I would think.

Ugh, I'm very sorry; I failed to describe my problem precisely. When I
wrote "cutting and pasting commit hashes from the git-blame output into
a git-rebase TODO list", I actually meant pasting the commit hash from
git-blame into the *search box* of the editor that keeps the git-rebase
TODO list open.

Basically I want to fix up a line of code in a patch in a longer patch
set, but don't know off-hand which patch in the set introduces that line
of code. So git-blame gives me a commit hash, and subsequently, I run
git-rebase, would like to jump to the TODO line with that commit hash,
and change the action from "pick" to "edit". And this *lookup* is what
fails, because the hash from git-blame is 13 nibbles long, but the
hashes in the git-rebase TODO list are 12 nibbles long. I always have to
remember to remove the last nibble in the search box; otherwise, there
is no match.

> FWIW, even if your core.abbrev says you want 12, if two objects share
> the same 12 hexdigits as the prefix, you do end up getting 13 or more,

Indeed, but that does not matter in practice (to me anyway); a 12-nibble
prefix length suffices for very large projects, and if ever there were a
collision, I'd just increase the length permanently to 13 nibbles. The
specific length is not relevant; agreement between git-blame's output
and everything else dealing with commit hashes in git is what I'd like
very much.

> so a parser that insists on exact 12 hexdigits sounds like a bug.
>
Apologies again, that (implied) bug existed only between my chair and
keyboard. :)

> Just for the sake of aesthetics, I agree that when we are not showing
> the boundary mark, it would make sense not to reserve one column that
> we know we will never use.  But unless there is a mistaken parser that
> insists on 12 hexdigits when 13 hexdigits you give uniquely identify
> the same object, I suspect you wouldn't even notice that the
> hexadecimal digits you see on the screen have one digit longer than
> usual ;-).

That's it precisely! :) I do *not* notice that the hash from git-blame
has one more hexadecimal character than the central abbrev setting; so I
just go ahead and blindly cut n' paste it, using the mouse, from the
terminal, to the editor search box that's displaying the git-rebase TODO
list. And then I don't understand why the lookup fails (until I realize
that I have forgotten, yet again, to strip the last nibble from the
commit hash coming from git-blame).

Once again, sorry about misstating the problem / use case!

Laszlo

Re: git-blame vs. abbrev

[Junio C Hamano]

Laszlo Ersek <laszlo.ersek@posteo.net> writes:

>> ... the same object, I suspect you wouldn't even notice that the
>> hexadecimal digits you see on the screen have one digit longer than
>> usual ;-).
>
> That's it precisely! :) I do *not* notice that the hash from git-blame
> has one more hexadecimal character than the central abbrev setting; so I
> just go ahead and blindly cut n' paste it, using the mouse, from the
> terminal, to the editor search box ...

Which means that there does not need any configuration variable to
trigger the new behaviour to protect your workflow from breaking, I
think, and instead we can just go ahead with the "-b then no extra
column for '^'" change, right? ;-)

Re: git-blame vs. abbrev

[Laszlo Ersek]

On 6/30/26 23:24, Junio C Hamano wrote:
> Laszlo Ersek <laszlo.ersek@posteo.net> writes:
> 
>>> ... the same object, I suspect you wouldn't even notice that the
>>> hexadecimal digits you see on the screen have one digit longer than
>>> usual ;-).
>>
>> That's it precisely! :) I do *not* notice that the hash from git-blame
>> has one more hexadecimal character than the central abbrev setting; so I
>> just go ahead and blindly cut n' paste it, using the mouse, from the
>> terminal, to the editor search box ...
> 
> Which means that there does not need any configuration variable to
> trigger the new behaviour to protect your workflow from breaking, I
> think, and instead we can just go ahead with the "-b then no extra
> column for '^'" change, right? ;-)

Yes, certainly!

Thank you!
Laszlo

Re: git-blame vs. abbrev

[René Scharfe]

On 6/30/26 9:49 PM, Junio C Hamano wrote:
> Laszlo Ersek <laszlo.ersek@posteo.net> writes:
> 
>> Hi,
>>
>> when git-blame is passed the "-b" option ("Show blank SHA-1 for boundary 
>> commits"), shouldn't git-blame *stop* reserving a commit hash nibble for 
>> the caret that otherwise marks boundary commits?
>>
>> More directly, I find it inconvenient that git-blame shows commit hashes 
>> that are one nibble longer (13) than my "core.abbrev" (12) setting; 
> 
> Just for the sake of aesthetics, I agree that when we are not
> showing the boundary mark, it would make sense not to reserve one
> column that we know we will never use.
Here's a patch to reserve a column only if marks are actually shown.

I strongly suspect that any line can only have a single mark, but I
didn't bother checking and proving whether that's actually true; the
new code should be able to handle multiple marks just fine.

Misses tests.

René


---
 builtin/blame.c | 61 ++++++++++++++++++++++++++++++++++---------------
 1 file changed, 42 insertions(+), 19 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index ffbd3ce5c5..0e747a43f2 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -453,6 +453,39 @@ static void determine_line_heat(struct commit_info *ci, const char **dest_color)
 	*dest_color = colorfield[i].col;
 }
 
+static size_t handle_marks(const struct blame_entry *ent, int opt, bool print)
+{
+	size_t len = 0;
+
+	if ((ent->suspect->commit->object.flags & UNINTERESTING) &&
+	    !blank_boundary && !(opt & OUTPUT_ANNOTATE_COMPAT)) {
+		if (print)
+			putchar('^');
+		len++;
+	}
+	if (mark_unblamable_lines && ent->unblamable) {
+		if (print)
+			putchar('*');
+		len++;
+	}
+	if (mark_ignored_lines && ent->ignored) {
+		if (print)
+			putchar('?');
+		len++;
+	}
+	return len;
+}
+
+static size_t print_marks(const struct blame_entry *ent, int opt)
+{
+	return handle_marks(ent, opt, true);
+}
+
+static size_t count_marks(const struct blame_entry *ent, int opt)
+{
+	return handle_marks(ent, opt, false);
+}
+
 static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent,
 		       int opt, struct blame_entry *prev_ent)
 {
@@ -499,23 +532,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(ent, opt);
 
 		printf("%.*s", (int)(length < GIT_MAX_HEXSZ ? length : GIT_MAX_HEXSZ), hex);
 		if (opt & OUTPUT_ANNOTATE_COMPAT) {
@@ -647,11 +667,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 +709,7 @@ 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 + max_marks_count;
 }
 
 static void sanity_check_on_fail(struct blame_scoreboard *sb, int baa)
-- 
2.54.0

Re: git-blame vs. abbrev

[Laszlo Ersek]

On 6/30/26 23:38, René Scharfe wrote:
> On 6/30/26 9:49 PM, Junio C Hamano wrote:
>> Laszlo Ersek <laszlo.ersek@posteo.net> writes:
>>
>>> Hi,
>>>
>>> when git-blame is passed the "-b" option ("Show blank SHA-1 for boundary
>>> commits"), shouldn't git-blame *stop* reserving a commit hash nibble for
>>> the caret that otherwise marks boundary commits?
>>>
>>> More directly, I find it inconvenient that git-blame shows commit hashes
>>> that are one nibble longer (13) than my "core.abbrev" (12) setting;
>>
>> Just for the sake of aesthetics, I agree that when we are not
>> showing the boundary mark, it would make sense not to reserve one
>> column that we know we will never use.
> Here's a patch to reserve a column only if marks are actually shown.
> 
> I strongly suspect that any line can only have a single mark, but I
> didn't bother checking and proving whether that's actually true; the
> new code should be able to handle multiple marks just fine.
> 
> Misses tests.
> 
> René
> 
> 
> ---
>   builtin/blame.c | 61 ++++++++++++++++++++++++++++++++++---------------
>   1 file changed, 42 insertions(+), 19 deletions(-)
> 
> diff --git a/builtin/blame.c b/builtin/blame.c
> index ffbd3ce5c5..0e747a43f2 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -453,6 +453,39 @@ static void determine_line_heat(struct commit_info *ci, const char **dest_color)
>   	*dest_color = colorfield[i].col;
>   }
>   
> +static size_t handle_marks(const struct blame_entry *ent, int opt, bool print)
> +{
> +	size_t len = 0;
> +
> +	if ((ent->suspect->commit->object.flags & UNINTERESTING) &&
> +	    !blank_boundary && !(opt & OUTPUT_ANNOTATE_COMPAT)) {
> +		if (print)
> +			putchar('^');
> +		len++;
> +	}
> +	if (mark_unblamable_lines && ent->unblamable) {
> +		if (print)
> +			putchar('*');
> +		len++;
> +	}
> +	if (mark_ignored_lines && ent->ignored) {
> +		if (print)
> +			putchar('?');
> +		len++;
> +	}
> +	return len;
> +}
> +
> +static size_t print_marks(const struct blame_entry *ent, int opt)
> +{
> +	return handle_marks(ent, opt, true);
> +}
> +
> +static size_t count_marks(const struct blame_entry *ent, int opt)
> +{
> +	return handle_marks(ent, opt, false);
> +}
> +
>   static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent,
>   		       int opt, struct blame_entry *prev_ent)
>   {
> @@ -499,23 +532,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(ent, opt);
>   
>   		printf("%.*s", (int)(length < GIT_MAX_HEXSZ ? length : GIT_MAX_HEXSZ), hex);
>   		if (opt & OUTPUT_ANNOTATE_COMPAT) {
> @@ -647,11 +667,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 +709,7 @@ 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 + max_marks_count;
>   }
>   
>   static void sanity_check_on_fail(struct blame_scoreboard *sb, int baa)

This way, I didn't even need to pass "-b". :)

I didn't try to test the patch exhaustively, but it definitely does what 
I'm after.

Tested-by: Laszlo Ersek <laszlo.ersek@posteo.net>

Thank you!
Laszlo

[PATCH] blame: reserve mark column only if necessary

[René Scharfe]

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>
---
 Documentation/git-blame.adoc | 11 +++---
 builtin/blame.c              | 68 ++++++++++++++++++++++++------------
 t/t8002-blame.sh             |  7 ++--
 3 files changed, 53 insertions(+), 33 deletions(-)

diff --git a/Documentation/git-blame.adoc b/Documentation/git-blame.adoc
index 8808009e87e..2b74e455997 100644
--- 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 ffbd3ce5c5a..5ae39d0458a 100644
--- 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 @@ int cmd_blame(int argc,
 	} 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 7822947f028..bf04b8273ef 100755
--- 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
-- 
2.55.0

Re: [PATCH] blame: reserve mark column only if necessary

[Junio C Hamano]

René Scharfe <l.s.r@web.de> writes:

>  `--abbrev=<n>`::
> +	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.

OK.

> +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;
> +}

Quite straight-forward.

> +static size_t count_marks(const struct blame_entry *ent, int opt)
> +{
> +	return print_marks(NULL, ent, opt);
> +}

OK.

> @@ -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 @@ int cmd_blame(int argc,
>  	} 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;

OK.


> diff --git a/t/t8002-blame.sh b/t/t8002-blame.sh
> index 7822947f028..bf04b8273ef 100755
> --- 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

OK.

>  	git blame -b --abbrev=10 ^HEAD -- abbrev.t >actual &&
>  	test_cmp expect actual