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, szager@xxxxxxxxxx wrote: >>> diff --git a/git-submodule.sh b/git-submodule.sh >>> index ab6b110..dcceb43 100755 >>> --- a/git-submodule.sh >>> +++ b/git-submodule.sh >>> @@ -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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html