Re: rev-list and "ambiguous" IDs

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 16, 2019, 03:47 UTC
Message-ID: <xmqqmucw4h4n.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <917e2664-6059-c190-30fd-02f3cf7aa5dc@virtuell-zuhause.de>

Thomas Braun <thomas.braun@virtuell-zuhause.de> writes:

Show 21 quoted lines

>> But if I choose another prefix that has no commits at all, it's
>> ambiguous under either, because the "committish" rule has no way to
>> decide:
>> 
>>   $ git show abcd2
>>   error: short SHA1 abcd2 is ambiguous
>>   hint: The candidates are:
>>   hint:   abcd22f55e tree
>>   hint:   abcd238df0 tree
>>   hint:   abcd2b1cc8 blob
>>   
>>   $ git log abcd2
>>   error: short SHA1 abcd2 is ambiguous
>>   hint: The candidates are:
>>   hint:   abcd22f55e tree
>>   hint:   abcd238df0 tree
>>   hint:   abcd2b1cc8 blob
>
> I would have expected that git log did just tell me that it could not
> find something commitish, instead it told me that there are multiple
> candidates, all of them being no commit.

With this, I 100% agree with.   The latter should instead say

    $ git log abcd2 [--]
    error: bad revision 'abcd2'

just like the case where no object has abcd2 as prefix.

When we ask for commit-ish or any specific type in general, there are a few possible cases.

 - There is only one such object that has the prefix and is
   compatible with the type.  We handle this correctly---yield that
   object and do not complain about ambiguity.

 - There are two or more such objects, or there is no such object.
   We show all objects that share the prefix, regardless of the
   type, which is way suboptimal.

An improvement can be localized to sha1-name.c::get_short_oid(), I would think. We know what type we want (e.g. GET_OID_COMMITTISH) in the function, so we should be able to teach collect_ambiguous() to discard an object with the given prefix but of a wrong type.