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.
