Re: [PATCH] git pull: Remove option handling done by fetch

From: Jens Lehmann <jens.lehmann@web.de>
Date: Feb 6, 2011, 21:59 UTC
Message-ID: <4D4F19D0.2000408@web.de>
In-Reply-To: <7vipwwx56r.fsf@alter.siamese.dyndns.org>

Am 06.02.2011 21:45, schrieb Junio C Hamano:

Show 9 quoted lines

> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> Yes, but isn't that exactly what the pull man-page says? Quote:
>> "Options meant for git pull itself and the underlying git merge
>> must be given before the options meant for git fetch."
> 
> Yes, it says that, and I think that was a weasely way to say "the command
> line parser in git-pull is broken".  The lines you are removing was from
> the patch that fixed that breakage, aren't they?

Nope, they were from the patch where I taught "git fetch" the "--recurse-submodules" option and was not aware at that time that "git pull" should just pass on almost all fetch options (the only exceptions to that rule being -q, -v, -n and --progress). The thing I had in mind was to later pass the same "--recurse-submodules" option to the merge command too (when I finished implementing that option). But when I understood later that pull handles the fetch options in an interesting way I noticed that it would depend on the order of options given if the "--recurse-submodules" would then be passed to both fetch and merge or just to fetch, which will lead to an interesting and unintuitive behavior I was not eager to expose.

So yes, I hit the strangeness of the "git pull" option parsing, but decided to not mess it up further by adding another option to the ones it does handle differently but play by the rules which are used now (The other possibility would have been to document it as a new option to "git pull", but that would have lead to the problem I described earlier when merge will learn that option too).

So I have no strong feelings about this patch but believe it is the right thing to do as long as "git pull" handles its options the way it does. But looking at the confusion that option handling caused I think it might be a worthwhile idea to overhaul it.

(CCed Jonathan, as he is the author of the lines I quoted)