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

["Jay Soffian" <jaysoffian@gmail.com>]

On Tue, Feb 26, 2008 at 4:23 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 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.

>  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.