Re: [PATCH] - Updated usage and simplified sub-command action invocation
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:44:04
imyousuf@gmail.com writes:
From: Imran M Yousuf <redacted> - 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.