Am 25.10.2012 12:45, schrieb W. Trevor King:
> On Thu, Oct 25, 2012 at 04:36:26AM -0400, Jeff King wrote:
>> On Wed, Oct 24, 2012 at 09:52:52PM -0700, wrote:
>>> diff --git a/ b/
>>> index ab6b110..dcceb43 100755
>>> --- a/
>>> +++ b/
>>> @@ -270,7 +270,6 @@ cmd_add()
>>>                     ;;
>>>             --reference=*)
>>>                     reference="$1"
>>> -                   shift
>>>                     ;;
>> Is that right? We'll unconditionally do a "shift" at the end of the
>> loop. If it were a two-part argument like "--reference foo", the extra
>> shift would make sense, but for "--reference=*", no extra shift should
>> be neccessary. Am I missing something?
> Both the patch and Jeff's analysis are right.  You only need an
> in-case shift if you consume "$2", or you're on ‘--’ and you're
> breaking before the end-of-case shift.

Right you are. The shift there is wrong, as there is no extra argument
to consume for "--reference=<repo>" (opposed to "--reference <repo>",
also see cmd_update() where this is done right).

So tested and Acked-By me, but me thinks the subject should read:

   [PATCH] submodule add: Fix handling of the --reference=<repo> option

and the commit message should begin with:

   Doing a shift there is wrong because there is no extra argument
   to consume when "--reference=<repo>" is used (note the '=' instead
   of a space).

Peff, is it ok for you to squash that in or do you want Stefan to resend?
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to
More majordomo info at

Reply via email to