Volume XXII, number 280Wednesday, October 7, 2026Latest message 51 minutes ago

The Git List

News and archive of git@vger.kernel.org, since April 2005

git-blame vs. abbrev

9 messages between Jun 30, 2026 and Jul 6, 2026, from Laszlo Ersek, Junio C Hamano, René Scharfe.

Plain Markdown or JSON for tools and agents.

Laszlo ErsekJun 30, 2026, 11:15 UTC on lore
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

Junio C HamanoJun 30, 2026, 19:49 UTC in reply to Laszlo Ersek on lore

Re: git-blame vs. abbrev

Laszlo Ersek <laszlo.ersek@posteo.net> writes:
Show 10 quoted lines
> 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 ;-).

Laszlo ErsekJun 30, 2026, 20:32 UTC in reply to Junio C Hamano on lore

Re: git-blame vs. abbrev

On 6/30/26 21:49, Junio C Hamano wrote:
Show 19 quoted lines
> 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. :)

Show 7 quoted lines
> 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
Junio C HamanoJun 30, 2026, 21:24 UTC in reply to Laszlo Ersek on lore

Re: git-blame vs. abbrev

Laszlo Ersek <laszlo.ersek@posteo.net> writes:
Show 8 quoted lines
>> ... 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? ;-)

René ScharfeJun 30, 2026, 21:38 UTC in reply to Junio C Hamano on lore

Re: git-blame vs. abbrev

On 6/30/26 9:49 PM, Junio C Hamano wrote:
Show 14 quoted lines
> 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
Laszlo ErsekJul 1, 2026, 08:43 UTC in reply to Junio C Hamano on lore

Re: git-blame vs. abbrev

On 6/30/26 23:24, Junio C Hamano wrote:
Show 15 quoted lines
> 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

Laszlo ErsekJul 1, 2026, 08:45 UTC in reply to René Scharfe on lore

Re: git-blame vs. abbrev

On 6/30/26 23:38, René Scharfe wrote:
Show 128 quoted lines
> 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

René ScharfeJul 6, 2026, 08:38 UTC in reply to Laszlo Ersek on lore

[PATCH] 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>
---
 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
Junio C HamanoJul 6, 2026, 20:33 UTC in reply to René Scharfe on lore

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

René Scharfe <l.s.r@web.de> writes:
Show 7 quoted lines
>  `--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.
Show 24 quoted lines
> +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.
Show 69 quoted lines
> @@ -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.
Show 24 quoted lines
> 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

Back to recent threads