Re: git-blame vs. abbrev

[Laszlo Ersek <laszlo.ersek@posteo.net>]

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