{"thread":{"id":"11562","subject":"[PATCH] - Updated usage and simplified sub-command action invocation","startedAt":"2008-01-10T04:07:25Z","lastAt":"2008-01-12T01:38:57Z","messageCount":7,"participants":["imyousuf@gmail.com","Junio C Hamano","Imran M Yousuf"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"64899","messageId":"1199938045-16289-1-git-send-email-imyousuf@gmail.com","threadId":"11562","inReplyTo":null,"subject":"[PATCH] - Updated usage and simplified sub-command action invocation","fromName":"","fromEmail":"imyousuf@gmail.com","sentAt":"2008-01-10T04:07:25Z","receivedAt":"2008-01-10T04:07:25Z","isPatch":true,"sender":{"key":"imyousuf@gmail.com","avatar":"https://gravatar.com/avatar/fda3c870262849d03c7b9c4d288842e128d6d80769fa7bc2d22731b7597928be?d=mp&s=160"},"body":"From: Imran M Yousuf <imyousuf@smartitengineering.com>\n\n- manual page of git-submodule and usage mentioned in git-subcommand.sh\nwere not same, thus synchronized them. In doing so also had to change the\nway the subcommands were parsed.\n\n- Previous version did not allow commands such as \"git-submodule add init\nupdate\". Thus not satisfying the following case -\n\nmkdir g; mkdir f; cd g/\ntouch g.txt; echo \"sample text for g.txt\" >> ./g.txt; git-init;\ngit-add g.txt; git-commit -a -m \"First commit on g\"\ncd ../f/; ln -s ../g/ init\ngit-init; git-submodule add init update;\ngit-commit -a -m \"With module update\"\nmkdir ../test; cd ../test\ngit-clone ../f/; cd f\ngit-submodule init update; git-submodule update update\ncd ../..; rm -rf ./f/ ./test/ ./g/\n\nThis patch fixes this issue and allows it as well.\n\n- Added 2 specific messages for usage error related to branch and cached\n\n- Simplified subcommand action invocation by simply invoking the action if\nall conditions are fulfilled. Excepting for parsing command line arguments\ncase statements are avoided and instead more direct if statement is\nintroduced.\n\nSigned-off-by: Imran M Yousuf <imyousuf@smartitengineering.com>\n---\n git-submodule.sh |   97 ++++++++++++++++++++++++++++++++++++------------------\n 1 files changed, 65 insertions(+), 32 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex ad9fe62..5d5e41d 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -4,18 +4,23 @@\n #\n # Copyright (c) 2007 Lars Hjemli\n \n-USAGE='[--quiet] [--cached] [add <repo> [-b branch]|status|init|update] [--] [<path>...]'\n+# Synopsis of this commands are as follows\n+# git-submodule [--quiet] [-b branch] add <repository> [<path>]\n+# git-submodule [--quiet] [--cached] [status] [--] [<path>...]\n+# git-submodule [--quiet] init [--] [<path>...]\n+# git-submodule [--quiet] update [--] [<path>...]\n+USAGE='[--quiet] [[[[-b branch] add <repo>]|[[[[--cached] status]|init|update] [--]]]  [<path>...]]'\n OPTIONS_SPEC=\n . git-sh-setup\n require_work_tree\n \n+MODULES_LIST='modules_list'\n+\n add=\n branch=\n-init=\n-update=\n-status=\n quiet=\n cached=\n+command=\n \n #\n # print stuff on stdout unless -q was specified\n@@ -293,20 +298,47 @@ modules_list()\n \tdone\n }\n \n+# If there is '--' as the first argument simply ignores it and thus shifts\n+check_for_terminator()\n+{\n+\tif test -n \"$1\" && test \"$1\" = \"--\"\n+\tthen\n+\t\tshift\n+\tfi\n+}\n+\n+# Command synopsis clearly shows that all arguments after\n+# subcommand are arguments to the command itself. Thus\n+# there lies no command that has configuration argument\n+# after the mention of the subcommand. Thus once the\n+# subcommand is found and the separator ('--') is ignored\n+# rest can be safely sent the subcommand action\n while test $# != 0\n do\n \tcase \"$1\" in\n \tadd)\n \t\tadd=1\n+\t\tcommand=\"module_$1\"\n+\t\tshift\n+\t\tbreak\n \t\t;;\n \tinit)\n-\t\tinit=1\n+\t\tcommand=\"modules_$1\"\n+\t\tshift\n+\t\tcheck_for_terminator \"$1\"\n+\t\tbreak\n \t\t;;\n \tupdate)\n-\t\tupdate=1\n+\t\tcommand=\"modules_$1\"\n+\t\tshift\n+\t\tcheck_for_terminator \"$1\"\n+\t\tbreak\n \t\t;;\n \tstatus)\n-\t\tstatus=1\n+\t\tcommand=\"$MODULES_LIST\"\n+\t\tshift\n+\t\tcheck_for_terminator \"$1\";\n+\t\tbreak\n \t\t;;\n \t-q|--quiet)\n \t\tquiet=1\n@@ -323,6 +355,9 @@ do\n \t\tcached=1\n \t\t;;\n \t--)\n+\t\t# It is shifted so that it is not passed\n+\t\t# as an argument to the default subcommand\n+\t\tshift\n \t\tbreak\n \t\t;;\n \t-*)\n@@ -335,30 +370,28 @@ do\n \tshift\n done\n \n-case \"$add,$branch\" in\n-1,*)\n-\t;;\n-,)\n-\t;;\n-,*)\n+# Throws usage error if branch is not used with add command\n+if test -n \"$branch\" &&\n+   test -z \"$add\"\n+then\n+\techo Branch can not be specified without add subcommand\n \tusage\n-\t;;\n-esac\n-\n-case \"$add,$init,$update,$status,$cached\" in\n-1,,,,)\n-\tmodule_add \"$@\"\n-\t;;\n-,1,,,)\n-\tmodules_init \"$@\"\n-\t;;\n-,,1,,)\n-\tmodules_update \"$@\"\n-\t;;\n-,,,*,*)\n-\tmodules_list \"$@\"\n-\t;;\n-*)\n+fi\n+\n+# If no command is specified than default command\n+# is - git submodule status\n+if test -z \"$command\"\n+then\n+\tcommand=\"$MODULES_LIST\"\n+fi\n+\n+# Throws usage if --cached is used by other than status, init or update\n+# that is used with add command\n+if test -n \"$cached\" &&\n+   test \"$command\" != \"$MODULES_LIST\"\n+then\n+\techo Cached can only be used with the status subcommand\n \tusage\n-\t;;\n-esac\n+fi\n+\n+\"$command\" \"$@\"\n-- \n1.5.3.7\n"},{"id":"64903","messageId":"7v8x2y8ahw.fsf@gitster.siamese.dyndns.org","threadId":"11562","inReplyTo":"1199938045-16289-1-git-send-email-imyousuf@gmail.com","subject":"Re: [PATCH] - Updated usage and simplified sub-command action invocation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-10T06:23:23Z","receivedAt":"2008-01-10T06:23:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"imyousuf@gmail.com writes:\n\n> From: Imran M Yousuf <imyousuf@smartitengineering.com>\n>\n> - manual page of git-submodule and usage mentioned in git-subcommand.sh\n> were not same, thus synchronized them. In doing so also had to change the\n> way the subcommands were parsed.\n>\n> - Previous version did not allow commands such as \"git-submodule add init\n> update\". Thus not satisfying the following case -\n>\n> mkdir g; mkdir f; cd g/\n> touch g.txt; echo \"sample text for g.txt\" >> ./g.txt; git-init;\n> git-add g.txt; git-commit -a -m \"First commit on g\"\n> cd ../f/; ln -s ../g/ init\n> git-init; git-submodule add init update;\n> git-commit -a -m \"With module update\"\n> mkdir ../test; cd ../test\n> git-clone ../f/; cd f\n> git-submodule init update; git-submodule update update\n> cd ../..; rm -rf ./f/ ./test/ ./g/\n\nI find this too verbose with too little information.\n\nIf I am reading you correctly, what you meant was that the way\ncommand parser was structured made subcommand names such as\n\"init\" and \"update\" reserved words, and it was impossible to use\nthem as arguments to commands.\n\nYou could have said something like this instead:\n\n\tThe command parser incorrectly made subcommand names to\n\tgit-submodule reserved, refusing them to be used as\n\tparameters to subcommands.  For example,\n\n        \t$ git submodule add init update\n\n\tto add a submodule whose (symbolic) name is \"init\" and\n\tthat resides at path \"update\" was refused.\n\nThat would have been much cleaner and easier on the reader than\nhaving to decipher what the 20+ command shell script sequence\nwas doing.\n\nI do agree that the breakage is worth fixing, though.\n\n> +# Synopsis of this commands are as follows\n> +# git-submodule [--quiet] [-b branch] add <repository> [<path>]\n> +# git-submodule [--quiet] [--cached] [status] [--] [<path>...]\n> +# git-submodule [--quiet] init [--] [<path>...]\n> +# git-submodule [--quiet] update [--] [<path>...]\n\nI somehow feel that syntactically the original implementation\nthat allowed subcommand specific options to come before the\nsubcommand name was a mistake.  It may be easier for users that\nboth \"-b branch add\" and \"add -b branch\" are accepted, but I\nhave to wonder if it would really hurt if we made \"-b branch\nadd\" a syntax error.\n\nSo how about reorganizing the top-level option parser like this:\n\n        while :\n        do\n                case $# in 0) break ;; esac\n                case \"$1\" in\n                add | status | init | update)\n                        # we have found subcommand.\n                        command=\"$1\"\n                        shift\n                        break ;;\n                --)\n                        # end of parameters\n                        shift\n                        break ;;\n                --quiet)\n                        quiet=1\n                        ;;\n                -*)\n                        die \"unknown option $1\"\n                esac\n                shift\n        done\n        test -n \"$command\" || command=$default_command\n        module_$command \"$@\"\n\nAnd then make individual command implementations responsible for \nparsing their own options (and perhaps the common ones, to allow\n\"git submodule add --quiet\", but that is optional), like:\n\n        module_add () {\n                while :\n                do\n                        case $# in 0) break ;; esac\n                        case \"$1\" in\n                        --cached)\n                                cached=1\n                                ;;\n                        -b | --branch)\n                                shift\n                                branch=\"$1\"\n                                test -n \"$branch\" ||\n                                die \"no branch name after -b?\"\n                                ;;\n                        --)\n                                shift\n                                break\n                                ;;\n                        --quiet)\n                                quiet=1\n                                ;;\n                        -*)\n                                die \"unknown option $1\"\n                        esac\n                        shift\n                done\n                repo=$1\n                path=$2\n                ...\n        }\n\nIn the above illustration I did not bother eliminating cut&paste\nduplication, but there may be a better way to share the piece to\nparse common options across subcommands option parsers and the\ntoplevel one.\n"},{"id":"64904","messageId":"7bfdc29a0801092251p3d46a3cau3db4d57c4f705043@mail.gmail.com","threadId":"11562","inReplyTo":"7v8x2y8ahw.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] - Updated usage and simplified sub-command action invocation","fromName":"Imran M Yousuf","fromEmail":"imyousuf@gmail.com","sentAt":"2008-01-10T06:51:42Z","receivedAt":"2008-01-10T06:51:42Z","isPatch":true,"sender":{"key":"imyousuf@gmail.com","avatar":"https://gravatar.com/avatar/fda3c870262849d03c7b9c4d288842e128d6d80769fa7bc2d22731b7597928be?d=mp&s=160"},"body":"On Jan 10, 2008 12:23 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> imyousuf@gmail.com writes:\n>\n> > From: Imran M Yousuf <imyousuf@smartitengineering.com>\n> >\n> > - manual page of git-submodule and usage mentioned in git-subcommand.sh\n> > were not same, thus synchronized them. In doing so also had to change the\n> > way the subcommands were parsed.\n> >\n> > - Previous version did not allow commands such as \"git-submodule add init\n> > update\". Thus not satisfying the following case -\n> >\n> > mkdir g; mkdir f; cd g/\n> > touch g.txt; echo \"sample text for g.txt\" >> ./g.txt; git-init;\n> > git-add g.txt; git-commit -a -m \"First commit on g\"\n> > cd ../f/; ln -s ../g/ init\n> > git-init; git-submodule add init update;\n> > git-commit -a -m \"With module update\"\n> > mkdir ../test; cd ../test\n> > git-clone ../f/; cd f\n> > git-submodule init update; git-submodule update update\n> > cd ../..; rm -rf ./f/ ./test/ ./g/\n>\n> I find this too verbose with too little information.\n>\n> If I am reading you correctly, what you meant was that the way\n> command parser was structured made subcommand names such as\n> \"init\" and \"update\" reserved words, and it was impossible to use\n> them as arguments to commands.\n>\n> You could have said something like this instead:\n>\n>         The command parser incorrectly made subcommand names to\n>         git-submodule reserved, refusing them to be used as\n>         parameters to subcommands.  For example,\n>\n>                 $ git submodule add init update\n>\n>         to add a submodule whose (symbolic) name is \"init\" and\n>         that resides at path \"update\" was refused.\n>\n\nI agree that your comment is better than, I will change it accordingly\nwhen resubmitting it.\n\n> That would have been much cleaner and easier on the reader than\n> having to decipher what the 20+ command shell script sequence\n> was doing.\n>\n> I do agree that the breakage is worth fixing, though.\n>\n> > +# Synopsis of this commands are as follows\n> > +# git-submodule [--quiet] [-b branch] add <repository> [<path>]\n> > +# git-submodule [--quiet] [--cached] [status] [--] [<path>...]\n> > +# git-submodule [--quiet] init [--] [<path>...]\n> > +# git-submodule [--quiet] update [--] [<path>...]\n>\n> I somehow feel that syntactically the original implementation\n> that allowed subcommand specific options to come before the\n> subcommand name was a mistake.  It may be easier for users that\n> both \"-b branch add\" and \"add -b branch\" are accepted, but I\n> have to wonder if it would really hurt if we made \"-b branch\n> add\" a syntax error.\n>\n\nI will recode it to have all options except for --quiet (which is\ninverse of -v or --verbose) be mentioned after the subcommand.\n\n> So how about reorganizing the top-level option parser like this:\n>\n>         while :\n>         do\n>                 case $# in 0) break ;; esac\n>                 case \"$1\" in\n>                 add | status | init | update)\n>                         # we have found subcommand.\n>                         command=\"$1\"\n>                         shift\n>                         break ;;\n>                 --)\n>                         # end of parameters\n>                         shift\n>                         break ;;\n>                 --quiet)\n>                         quiet=1\n>                         ;;\n>                 -*)\n>                         die \"unknown option $1\"\n>                 esac\n>                 shift\n>         done\n>         test -n \"$command\" || command=$default_command\n>         module_$command \"$@\"\n>\n\nActually module_$command is not possible because only add's module is\nmodule_add rest are modules_$command. Thus I would require another if\nelse and that was the original reason for not using it. Instead I\nshould have (and will) used -\n\n       case \"$1\" in\n       add)\n               add=1\n               command=\"module_$1\"\n               shift\n               break\n               ;;\n       init|update|status)\n               init=1\n               command=\"modules_$1\"\n               shift\n               check_for_terminator \"$1\"\n               break\n               ;;\n\n> And then make individual command implementations responsible for\n> parsing their own options (and perhaps the common ones, to allow\n> \"git submodule add --quiet\", but that is optional), like:\n>\n>         module_add () {\n>                 while :\n>                 do\n>                         case $# in 0) break ;; esac\n>                         case \"$1\" in\n>                         --cached)\n>                                 cached=1\n>                                 ;;\n>                         -b | --branch)\n>                                 shift\n>                                 branch=\"$1\"\n>                                 test -n \"$branch\" ||\n>                                 die \"no branch name after -b?\"\n>                                 ;;\n>                         --)\n>                                 shift\n>                                 break\n>                                 ;;\n>                         --quiet)\n>                                 quiet=1\n>                                 ;;\n>                         -*)\n>                                 die \"unknown option $1\"\n>                         esac\n>                         shift\n>                 done\n>                 repo=$1\n>                 path=$2\n>                 ...\n>         }\n>\n> In the above illustration I did not bother eliminating cut&paste\n> duplication, but there may be a better way to share the piece to\n> parse common options across subcommands option parsers and the\n> toplevel one.\n>\n\nAs add subcommand does not support --cached it should be considered in\n-*, just mentioning for your FYI, I got the point of module parsing\ntheir own arguments and I am in agreement.\n\n>\n\nI will make the necessary changes and resubmit the patch tomorrow.\n\nBest regards,\n\n-- \nImran M Yousuf\n"},{"id":"64906","messageId":"7vzlve6t69.fsf@gitster.siamese.dyndns.org","threadId":"11562","inReplyTo":"7bfdc29a0801092251p3d46a3cau3db4d57c4f705043@mail.gmail.com","subject":"Re: [PATCH] - Updated usage and simplified sub-command action invocation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-10T07:22:54Z","receivedAt":"2008-01-10T07:22:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Imran M Yousuf\" <imyousuf@gmail.com> writes:\n\n> On Jan 10, 2008 12:23 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> ...\n>> I somehow feel that syntactically the original implementation\n>> that allowed subcommand specific options to come before the\n>> subcommand name was a mistake.  It may be easier for users that\n>> both \"-b branch add\" and \"add -b branch\" are accepted, but I\n>> have to wonder if it would really hurt if we made \"-b branch\n>> add\" a syntax error.\n>\n> I will recode it to have all options except for --quiet (which is\n> inverse of -v or --verbose) be mentioned after the subcommand.\n\nJust a word of caution when dealing with me.\n\nUnlike Linus, I am not always right.  And other people on the\nlist who are here longer already know this. I am reasonably sure\nthat some of them will disagree with me on design issues like\nthis one; I mildly suspect that this forbidding \"-b branch add\"\nmight be met with resistance from existing users.\n\nYou do not have to agree with me on every little detail I\nmention.  If you feel a design issue might be contentious, it\ncould turn out to be a better use of your time to keep the code\nas it is while waiting to see if other people would offer better\nalternatives.\n\n> Actually module_$command is not possible because only add's module is\n> module_add rest are modules_$command....\n\nIs there a fundamental reason why you cannot rename them to be\nmore consistent?\n"},{"id":"64907","messageId":"7bfdc29a0801092341j60dcb081xe4bf6c22cbaf30f2@mail.gmail.com","threadId":"11562","inReplyTo":"7vzlve6t69.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] - Updated usage and simplified sub-command action invocation","fromName":"Imran M Yousuf","fromEmail":"imyousuf@gmail.com","sentAt":"2008-01-10T07:41:08Z","receivedAt":"2008-01-10T07:41:08Z","isPatch":true,"sender":{"key":"imyousuf@gmail.com","avatar":"https://gravatar.com/avatar/fda3c870262849d03c7b9c4d288842e128d6d80769fa7bc2d22731b7597928be?d=mp&s=160"},"body":"On Jan 10, 2008 1:22 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> \"Imran M Yousuf\" <imyousuf@gmail.com> writes:\n>\n> > On Jan 10, 2008 12:23 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> > ...\n> >> I somehow feel that syntactically the original implementation\n> >> that allowed subcommand specific options to come before the\n> >> subcommand name was a mistake.  It may be easier for users that\n> >> both \"-b branch add\" and \"add -b branch\" are accepted, but I\n> >> have to wonder if it would really hurt if we made \"-b branch\n> >> add\" a syntax error.\n> >\n> > I will recode it to have all options except for --quiet (which is\n> > inverse of -v or --verbose) be mentioned after the subcommand.\n>\n> Just a word of caution when dealing with me.\n>\n> Unlike Linus, I am not always right.  And other people on the\n\nI will cautiously remember the caution :).\n\n> list who are here longer already know this. I am reasonably sure\n> that some of them will disagree with me on design issues like\n> this one; I mildly suspect that this forbidding \"-b branch add\"\n> might be met with resistance from existing users.\n>\n> You do not have to agree with me on every little detail I\n> mention.  If you feel a design issue might be contentious, it\n> could turn out to be a better use of your time to keep the code\n> as it is while waiting to see if other people would offer better\n> alternatives.\n\nActually the best design, IMHO, is to have separate commands itself\nfor them, that is submodule-add, submodule-init, submodule-update,\nsubmodule-status or submodule. I think this would also make it\ncoherent with other commands such as git-ls, git-merge, git-show. In\nthat way we could have a common .sh file that will contain the common\nfunctions and can be accessed from the command shell scripts. This\nwould also make it quite simple.\n\n>\n> > Actually module_$command is not possible because only add's module is\n> > module_add rest are modules_$command....\n>\n> Is there a fundamental reason why you cannot rename them to be\n> more consistent?\n\nIn fact it is consistent, add works on a single module only, whereas\nrest of the command works either on 1 or more. Thus having plural\n(modules) is logical.\n\n>\n\nBest regards,\n\n-- \nImran M Yousuf\n"},{"id":"65000","messageId":"7bfdc29a0801110109v10135afmd57c604e0d23250d@mail.gmail.com","threadId":"11562","inReplyTo":"7v8x2y8ahw.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] - Updated usage and simplified sub-command action invocation","fromName":"Imran M Yousuf","fromEmail":"imyousuf@gmail.com","sentAt":"2008-01-11T09:09:29Z","receivedAt":"2008-01-11T09:09:29Z","isPatch":true,"sender":{"key":"imyousuf@gmail.com","avatar":"https://gravatar.com/avatar/fda3c870262849d03c7b9c4d288842e128d6d80769fa7bc2d22731b7597928be?d=mp&s=160"},"body":"On Jan 10, 2008 12:23 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> imyousuf@gmail.com writes:\n>\n> > From: Imran M Yousuf <imyousuf@smartitengineering.com>\n> >\n> > - manual page of git-submodule and usage mentioned in git-subcommand.sh\n> > were not same, thus synchronized them. In doing so also had to change the\n> > way the subcommands were parsed.\n> >\n> > - Previous version did not allow commands such as \"git-submodule add init\n> > update\". Thus not satisfying the following case -\n> >\n> > mkdir g; mkdir f; cd g/\n> > touch g.txt; echo \"sample text for g.txt\" >> ./g.txt; git-init;\n> > git-add g.txt; git-commit -a -m \"First commit on g\"\n> > cd ../f/; ln -s ../g/ init\n> > git-init; git-submodule add init update;\n> > git-commit -a -m \"With module update\"\n> > mkdir ../test; cd ../test\n> > git-clone ../f/; cd f\n> > git-submodule init update; git-submodule update update\n> > cd ../..; rm -rf ./f/ ./test/ ./g/\n>\n> I find this too verbose with too little information.\n>\n> If I am reading you correctly, what you meant was that the way\n> command parser was structured made subcommand names such as\n> \"init\" and \"update\" reserved words, and it was impossible to use\n> them as arguments to commands.\n>\n> You could have said something like this instead:\n>\n>         The command parser incorrectly made subcommand names to\n>         git-submodule reserved, refusing them to be used as\n>         parameters to subcommands.  For example,\n>\n>                 $ git submodule add init update\n>\n>         to add a submodule whose (symbolic) name is \"init\" and\n>         that resides at path \"update\" was refused.\n>\n> That would have been much cleaner and easier on the reader than\n> having to decipher what the 20+ command shell script sequence\n> was doing.\n>\n> I do agree that the breakage is worth fixing, though.\n>\n> > +# Synopsis of this commands are as follows\n> > +# git-submodule [--quiet] [-b branch] add <repository> [<path>]\n> > +# git-submodule [--quiet] [--cached] [status] [--] [<path>...]\n> > +# git-submodule [--quiet] init [--] [<path>...]\n> > +# git-submodule [--quiet] update [--] [<path>...]\n>\n> I somehow feel that syntactically the original implementation\n> that allowed subcommand specific options to come before the\n> subcommand name was a mistake.  It may be easier for users that\n> both \"-b branch add\" and \"add -b branch\" are accepted, but I\n> have to wonder if it would really hurt if we made \"-b branch\n> add\" a syntax error.\n\nJust a point in this regard, if we allow to have both \"-b branch add\"\nand \"add -b branch\" in that case there will be code redundancy as\nthere will have to parsing of \"-b branch\" in the subcommand parsing\nand in the command module (module_add). I will be implementing now, to\nsupport both; with the exception for --quiet.\n\n>\n> So how about reorganizing the top-level option parser like this:\n>\n>         while :\n>         do\n>                 case $# in 0) break ;; esac\n>                 case \"$1\" in\n>                 add | status | init | update)\n>                         # we have found subcommand.\n>                         command=\"$1\"\n>                         shift\n>                         break ;;\n>                 --)\n>                         # end of parameters\n>                         shift\n>                         break ;;\n>                 --quiet)\n>                         quiet=1\n>                         ;;\n>                 -*)\n>                         die \"unknown option $1\"\n>                 esac\n>                 shift\n>         done\n>         test -n \"$command\" || command=$default_command\n>         module_$command \"$@\"\n>\n> And then make individual command implementations responsible for\n> parsing their own options (and perhaps the common ones, to allow\n> \"git submodule add --quiet\", but that is optional), like:\n>\n>         module_add () {\n>                 while :\n>                 do\n>                         case $# in 0) break ;; esac\n>                         case \"$1\" in\n>                         --cached)\n>                                 cached=1\n>                                 ;;\n>                         -b | --branch)\n>                                 shift\n>                                 branch=\"$1\"\n>                                 test -n \"$branch\" ||\n>                                 die \"no branch name after -b?\"\n>                                 ;;\n>                         --)\n>                                 shift\n>                                 break\n>                                 ;;\n>                         --quiet)\n>                                 quiet=1\n>                                 ;;\n>                         -*)\n>                                 die \"unknown option $1\"\n>                         esac\n>                         shift\n>                 done\n>                 repo=$1\n>                 path=$2\n>                 ...\n>         }\n>\n> In the above illustration I did not bother eliminating cut&paste\n> duplication, but there may be a better way to share the piece to\n> parse common options across subcommands option parsers and the\n> toplevel one.\n>\n>\n\n\n\n-- \nImran M Yousuf\n"},{"id":"65105","messageId":"7vk5mfzutq.fsf@gitster.siamese.dyndns.org","threadId":"11562","inReplyTo":"7bfdc29a0801092341j60dcb081xe4bf6c22cbaf30f2@mail.gmail.com","subject":"Re: [PATCH] - Updated usage and simplified sub-command action invocation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-12T01:38:57Z","receivedAt":"2008-01-12T01:38:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Imran M Yousuf\" <imyousuf@gmail.com> writes:\n\n>> > Actually module_$command is not possible because only add's module is\n>> > module_add rest are modules_$command....\n>>\n>> Is there a fundamental reason why you cannot rename them to be\n>> more consistent?\n>\n> In fact it is consistent, add works on a single module only, whereas\n> rest of the command works either on 1 or more. Thus having plural\n> (modules) is logical.\n\nIt certainly is consistent in _that_ meaning of the word, but I\nwas not talking about that consistency, which is less useful in\nthis context.\n\nThe consistency I was talking about was \"A subcommand called $foo\nis always handled by a shell function called cmd_$foo\".  That is\nalso a consistency, and it is of much more useful kind in a\nsituation like this, namely, a command dispatcher.\n\nIf you have show_blobs() and show_commit() subroutines, former\nof which takes 1 or more blobs while the latter of which can\nonly take 1 commit, being consistent in your meaning might help\nthe programmers avoiding a mistake to pass two or more commits\nto a non-existent show_commits().  In that sense, your kind of\nconsistency is not totally useless.\n\nHowever, it is not so useful in a context where there is one\ncall site for each of the functions, like a command dispatcher\nscenario.\n"}]}