Re: [PATCH] checkout: swap the order of ambiguity check for :/ syntax

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 25, 2016, 17:19 UTC
Message-ID: <xmqq60qoojf7.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CACsJy8AnDoh-CZp8oa21YHcM3sxoTJZiK7fP+ecNyqd0-DBZAA@mail.gmail.com>

Duy Nguyen <pclouds@gmail.com> writes:

Show 18 quoted lines

> On Wed, Aug 24, 2016 at 11:35 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>>
>>> It's not wonderful, but it's in line with how git-checkout stops caring
>>> about ambiguity after the first argument can be resolved as a ref
>>> (there's even a test for it, t2010.6).
>>
>> But that is justifiable because checkout can only ever take one
>> revision.  What follows, if there are any, must be paths, and more
>> importantly, it would be perfectly reasonable if some of them were
>> missing in the working tree ("ow, I accidentally removed that file,
>> I need to resurrect it from the index").  Does the same justification
>> apply to this change?
>
> I think there is a misunderstanding. My "after" is in "after the first
> argument can be resolved, check if it exists in worktree too, if so
> it's ambiguous and bail". This is usually how we detect ambiguation.
> But git-checkout does not do the "check if it exists..." clause.

Hmph.  The "case 4" in the function you touched says

         * case 4: git checkout <something> <paths>
         *
         *   The first argument must not be ambiguous.
         *   - If it's *only* a reference, treat it like case (1).
         *   - If it's only a path, treat it like case (2).
         *   - else: fail.

Did we break it recently?