Subject: Re: [PATCH 10/20] diff-parseopt: convert --[no-]abbrev

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Thu, 21 Mar 2019 00:00:21 +0100

On Wed, Mar 20 2019, Nguyễn Thái Ngọc Duy wrote:

> [...]And the '40' change is self explanatory.

Let me make an attempt at being dense anyway...

> -		else if (v > 40)
> -			v = 40;
> +		else if (v > the_hash_algo->hexsz)
> +			v = the_hash_algo->hexsz;
>  	}

This is obviously not a regression, it's a hardcoded 40 *now*. So we
should take this patch.

But in general, I wonder how this is going to work once we get a few
steps further into the SHA-256 migration. I.e. here we're still parsing
the command-line, and the_hash_algo might be initialized early to SHA-1.

So if I set --abbrev=45 it'll be trimmed to --abbrev=40 by this code.

But then shortly afterwards we pass my SHA-256 object down to some
machinery, and will then want to abbreviate it.

Isn't that part of the code something we're going to want to support
looking up objects in either hash, even if we initially started out with
SHA-1 in the_hash_algo? So we'll be over-abbreviating a SHA-256 object.

Leaving aside the sillyness of wanting to abbreviate *anything* to 45
characters, I wonder how those sorts of chicken & egg hash scenarios
will go involving the_hash_algo.