Re: [PATCH] find_unique_abbrev(): honor caller-supplied "len" better

Linus Torvalds [off-list ref] writes:

When you pass in 6 as a len, and that isn't sufficient, it expands it
to (say) 10. And then you pass in 7 as a length, and now it's
sufficient, so it keeps it at 7.

Hmph, that is not the way I wanted to do things.  If len is 6 and the
result needs to be 10 to be sufficient, it should still yield 10 if the
len you give is 7 (or 8 or 9 or 10).  Of course it would be stupid if you
don't do it that way.

I thought the code should work that way already (unless of course there is
an implementation bug).

How many characters do we need to name master uniquely today?

    $ ./git -c core.abbrevguard=0 rev-parse --short=1 master
    83c3c

Ok, so there are more than one object with 83c3 and 83c3c is the absolute
minimum.  We can ask for extra for futureproofing.

    $ ./git -c core.abbrevguard=3 rev-parse --short=1 master
    83c3c622

And we get three extra and the result is 8 characters long.  What if we
give len=7 that is still short?

    $ ./git -c core.abbrevguard=3 rev-parse --short=7 master
    83c3c622

Of course we allow the limit 7 to be busted to keep the futureproofing.

    $ ./git -c core.abbrevguard=3 rev-parse --short=8 master
    83c3c622

Obeying 8 doesn't hurt the futureproofing so the output remains the same.
But if you say len=9, you get 9

    $ ./git -c core.abbrevguard=3 rev-parse --short=9 master
    83c3c6222

even though we know 8 is plenty futureproof, but you get 9 because you
asked for 9.

What am I missing to be called "just stupid" and "crazy"?

The earlier code was adding the three extra over what was asked for.  With
guard=3 and len=7, you would have got 10 for this object that only needs 5
as absolute minimum today, and giving 8 would have been enough to stay
unique with room for 3 extra characters to grow.  That is what I was
fixing.

 (b) You can't change DEFAULT_ABBREV except with the command line option.

I would agree that would need changing, but I think that is more or less
an independent issue.

I thought I took your patch to introduce the configuration when I sent the
abbrevguard as a companion patch, but apparently I didn't.  That would fix
the "grep DEFAULT_ABBREV" issue.