Jeff King <p...@peff.net> writes:
>> > + case "$1" in
>> > + -j)
>> > + parallel=${1#-j} ;;
>>
>> This is curious. Did you mean "-j*)" on the line above this one?
>
> Hmph, yes, I think this was broken even in the original. And after going
> through "rev-parse --parseopt", we should have a separate argument
> anyway, even for the "stuck" form. Worse, the OPTIONS_SPEC doesn't
> mention the argument, so it barfs on "-j4".
Ah, I forgot (just like somebody else, and even worse is that I
reminded him of this fact that I am forgetting here) that we use the
parseopt thing to normalize the option parsing, so with the correct
spec "-j)" is the right thing to say (but yes then you'd look at $2
and then shift both aawy).
> I think we need to squash this in:
Yup. Thanks.
> diff --git a/Documentation/doc-diff b/Documentation/doc-diff
> index 5d5b243384..f483fe427c 100755
> --- a/Documentation/doc-diff
> +++ b/Documentation/doc-diff
> @@ -3,7 +3,7 @@
> OPTIONS_SPEC="\
> doc-diff [options] <from> <to> [-- <diff-options>]
> --
> -j parallel argument to pass to make
> +j=n parallel argument to pass to make
> f force rebuild; do not rely on cached results
> "
> SUBDIRECTORY_OK=1
> @@ -15,7 +15,7 @@ while test $# -gt 0
> do
> case "$1" in
> -j)
> - parallel=${1#-j} ;;
> + parallel=$2; shift ;;
> -f)
> force=t ;;
> --)
>
>> Then "script -j" (no explicit number) would get here and autodetect.
>> Running the script without any "-j" would also get an empty parallel
>> and come here.
>
> Yeah, I think that is the wrong thing. If anything "-j" should behave
> like "make -j". However, it looks like "rev-parse --parseopt" doesn't
> play well with optional arguments for short-only options. You get "-j",
> but then you have no idea whether the next argument is an optional value
> for it, or another option entirely. Arguably it should give a blank
> string or something (if you have long options, then it uses the
> long-stock form, which is fine).
>
>> So "script -j1" would be how a user would say "I want to use exactly
>> one process, not any parallelism", which makes sense.
>
> Right, that was the thing I actually wanted to have happen. :)
>
> -Peff
