Re: [PATCH] - Updated usage and simplified sub-command action invocation

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

imyousuf@gmail.com writes:

> From: Imran M Yousuf <imyousuf@smartitengineering.com>
>
> - manual page of git-submodule and usage mentioned in git-subcommand.sh
> were not same, thus synchronized them. In doing so also had to change the
> way the subcommands were parsed.
>
> - Previous version did not allow commands such as "git-submodule add init
> update". Thus not satisfying the following case -
>
> mkdir g; mkdir f; cd g/
> touch g.txt; echo "sample text for g.txt" >> ./g.txt; git-init;
> git-add g.txt; git-commit -a -m "First commit on g"
> cd ../f/; ln -s ../g/ init
> git-init; git-submodule add init update;
> git-commit -a -m "With module update"
> mkdir ../test; cd ../test
> git-clone ../f/; cd f
> git-submodule init update; git-submodule update update
> cd ../..; rm -rf ./f/ ./test/ ./g/

I find this too verbose with too little information.

If I am reading you correctly, what you meant was that the way
command parser was structured made subcommand names such as
"init" and "update" reserved words, and it was impossible to use
them as arguments to commands.

You could have said something like this instead:

	The command parser incorrectly made subcommand names to
	git-submodule reserved, refusing them to be used as
	parameters to subcommands.  For example,

        	$ git submodule add init update

	to add a submodule whose (symbolic) name is "init" and
	that resides at path "update" was refused.

That would have been much cleaner and easier on the reader than
having to decipher what the 20+ command shell script sequence
was doing.

I do agree that the breakage is worth fixing, though.

> +# Synopsis of this commands are as follows
> +# git-submodule [--quiet] [-b branch] add <repository> [<path>]
> +# git-submodule [--quiet] [--cached] [status] [--] [<path>...]
> +# git-submodule [--quiet] init [--] [<path>...]
> +# git-submodule [--quiet] update [--] [<path>...]

I somehow feel that syntactically the original implementation
that allowed subcommand specific options to come before the
subcommand name was a mistake.  It may be easier for users that
both "-b branch add" and "add -b branch" are accepted, but I
have to wonder if it would really hurt if we made "-b branch
add" a syntax error.

So how about reorganizing the top-level option parser like this:

        while :
        do
                case $# in 0) break ;; esac
                case "$1" in
                add | status | init | update)
                        # we have found subcommand.
                        command="$1"
                        shift
                        break ;;
                --)
                        # end of parameters
                        shift
                        break ;;
                --quiet)
                        quiet=1
                        ;;
                -*)
                        die "unknown option $1"
                esac
                shift
        done
        test -n "$command" || command=$default_command
        module_$command "$@"

And then make individual command implementations responsible for 
parsing their own options (and perhaps the common ones, to allow
"git submodule add --quiet", but that is optional), like:

        module_add () {
                while :
                do
                        case $# in 0) break ;; esac
                        case "$1" in
                        --cached)
                                cached=1
                                ;;
                        -b | --branch)
                                shift
                                branch="$1"
                                test -n "$branch" ||
                                die "no branch name after -b?"
                                ;;
                        --)
                                shift
                                break
                                ;;
                        --quiet)
                                quiet=1
                                ;;
                        -*)
                                die "unknown option $1"
                        esac
                        shift
                done
                repo=$1
                path=$2
                ...
        }

In the above illustration I did not bother eliminating cut&paste
duplication, but there may be a better way to share the piece to
parse common options across subcommands option parsers and the
toplevel one.