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

imyousuf@gmail.com writes:

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.

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.