Re: [PATCH v2] pull: document usage via OPTIONS_SPEC

From: Jay Soffian <jaysoffian@gmail.com>
Date: Feb 26, 2008, 21:37 UTC
Message-ID: <76718490802261337i16e5f27bg11cf452fdc0542ca@mail.gmail.com>
In-Reply-To: <7v4pbv4dkw.fsf@gitster.siamese.dyndns.org>

On Tue, Feb 26, 2008 at 4:23 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 9 quoted lines

> Jay Soffian <jaysoffian@gmail.com> writes:
>
>  >  * There is one semantic change. You can't use "-s=<strategy>" anymore. That's not
>  >    really proper usage of a short option (it's either "-s<strategy>" or
>  >    "-s <strategy>"). Is it okay to not accept the "-s=<strategy>" form?
>
>  Well, with my maintainer hat on, I must resist _any_ change ;-).
>  Personally I would not mind this.  It is a borderline between
>  regression and making the option parameters more consistent.

My reasoning was that were the script converted to a built-in, the -s= behavior would not likely be maintained (or even noticed...). And that form certainly isn't documented. I think it was an accident that it ever worked.

>  If a contributor feels wasting his time, what should reviewers
>  feel reviewing such patches ;-)?

I get the smiley, but let me rephrase: is there a long term goal to replace all shell scripts with builtins?

And to answer your rhetorical question, reviewers should feel appreciated, because they are.

>  While it is technically correct that you _can_ feed any option
>  meant for git-fetch to this command, some options do not make
>  any sense in the context of git-pull, and we should not
>  advertise them, or better yet, actively reject them if you can.

Well then we have a documentation issue then, because it was from the git pull docs that I wrote the option spec. So I suppose this patch should include a documentation cleanup at the same time.

Show 7 quoted lines

>  Because the loop breaks here, the option description above
>  should mention that options meant for fetch should come after
>  all the options you want to give to git-pull itself.  For
>  example, I do not think "git pull -q -s stupid $there $that" is
>  meant to work.
>
>  A better long-term alternative would be to lift that restriction.

Sigh, nothing is ever as simple as it seems. And here I was just trying to improve the usage statement. :-(

>  I do not recall offhand but does the parse-options reorder the
>  options in any way?  If that is the case, it makes the above not
>  a long-term thing but a must-be-done in a patch that starts to
>  use parse-options.

I don't think it does.

Back to the drawing board...

j.