Re: [PATCH 2/2] blame: introduce -u/--unique option

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 9, 2011, 19:45 UTC
Message-ID: <7vy64o9ixz.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1299581951-4670-2-git-send-email-namhyung@gmail.com>

Namhyung Kim <namhyung@gmail.com> writes:

> -u/--unique option will find and use minimum length of unique
> SHA-1 name. If -l option is specified also, it will have higher
> priority, IOW git blame will use full 40-length SHA-1 name.

Show 8 quoted lines

> @@ -1867,6 +1878,10 @@ static void find_alignment(struct scoreboard *sb, int *option)
>  			longest_dst_lines = num;
>  		if (largest_score < ent_score(sb, e))
>  			largest_score = ent_score(sb, e);
> +		sha1 = find_unique_abbrev(suspect->commit->object.sha1,
> +					  MINIMUM_ABBREV);
> +		if (longest_uniq_sha1 < strlen(sha1))
> +			longest_uniq_sha1 = strlen(sha1);

The logic to determine and keep track of the longuest-unique looks correct, but I was hoping that we already have an easy optimization codepath to do this only once per commit, not for every blame-entry in the result. Doesn't the code have a similar optimization to figure out the necessary number of columns to show author names (I haven't read the code recently, though)?

Also we might find that the performance impact of doing this may be so miniscule that it is not worth wasting a short option name. If we were to use an option, I was actually hoping that the option would let the users specify a value different from the hardcoded 8 at the same time. E.g.

    git blame --abbrev=8 ;# current default with uniquefy applied
    git blame --abbrev=4 ;# equivalent to your "blame -u"

Can we have a benchmark of this feature in a largish and busy file in a project with a deep history?