Re: git-blame vs. abbrev

["René Scharfe" <l.s.r@web.de>]

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