Re: [PATCH 3/3] git-submodule.sh: improve parsing of short options

[Junio C Hamano <gitster@pobox.com>]

Roy Eldar <royeldar0@gmail.com> writes:

> Some command-line options have a short form which takes an argument; for
> example, "--jobs" has the form "-j", and it takes a numerical argument.
>
> When parsing short options, support the case where there is no space
> between the flag and the option argument, in order to improve
> consistency with the rest of the builtin git commands.
>
> Signed-off-by: Roy Eldar <royeldar0@gmail.com>
> ---
>  git-submodule.sh | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index a47d2a89f3..fc85458fb1 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -77,6 +77,9 @@ cmd_add()
>  			branch=$2
>  			shift
>  			;;
> +		-b*)
> +			branch="${1#-b}"
> +			;;
>  		--branch=*)
>  			branch="${1#--branch=}"
>  			;;

OK, so we used to take "--branch foo" and assign 'foo' to $branch,
"--branch=foo" now does the same, and then we have "-b foo" doing
the same with this step.

> @@ -352,6 +355,9 @@ cmd_update()
>  			jobs="--jobs=$2"
>  			shift
>  			;;
> +		-j*)
> +			jobs="--jobs=${1#-j}"
> +			;;
>  		--jobs=*)
>  			jobs=$1
>  			;;

This is a bit curious.  If you stick the principle in 1/3, this
should assign "4", not "--jobs=4", to variable $jobs upon seeing
"-j4", and that would be consistent with how the $branch gets its
value above.

As I said in the devil's advocate section in my review for 1/3, I
often find the value of the variable spelling out the option name
as well as the option value (i.e., force="--force", not force=1;
branch="--branch=foo", not branch=foo; jobs=--jobs=4, not jobs=4)
easier to debug and drive other programs using these variables, so I
do not mind jobs=--jobs=4 at all, but if we want to be consistent in
the other direction, this would probably want to be modified in the
name of consistency?

Other than that, all three patches looked sane to me.

Thanks.