Re: [PATCH] add a script to diff rendered documentation
From: Junio C Hamano <hidden>
Date: 2018-08-06 19:29:32
Jeff King [off-list ref] writes:
quoted
quoted
+ 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.
quoted hunk
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 ;; --)quoted
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).quoted
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