{"thread":{"id":"11614","subject":"[PATCH] - git submodule subcommand parsing modified.","startedAt":"2008-01-14T03:22:36Z","lastAt":"2008-01-16T20:08:59Z","messageCount":7,"participants":["imyousuf@gmail.com","Junio C Hamano","Imran M Yousuf"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"65293","messageId":"1200280956-19920-1-git-send-email-imyousuf@gmail.com","threadId":"11614","inReplyTo":null,"subject":"[PATCH] - git submodule subcommand parsing modified.","fromName":"","fromEmail":"imyousuf@gmail.com","sentAt":"2008-01-14T03:22:36Z","receivedAt":"2008-01-14T03:22:36Z","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\n\tgit-submodule add init update\nas the command parser incorrectly made subcommand names reserve.\nThus refusing them to be used as parameters to subcommands. As a result it\nwas impossible to add a submodule whose (symbolic) name is \"init\" and that\nresides at path \"update\" was refused. For more details the following case\ncan be considered -\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- Status currently is implemented to show list only but later\nimplementation might change and list and status could coexists. Thus\nstatus module is introduced. The module is also used to parse its\narguments\n\n- Subcommands will also parse their own commands; thus enabling command\nspecific arguments to be passed after the command. For example,\n\tgit-submodule -q add -b master module_a\n\tgit-submodule -q status -c\nIt is to be noted that -q or --quiet is specified before the subcommand\nsince it is for the submodule command in general rather than the\nsubcommand. It is mention worthy that backward compatibility exists and\nthus commands like git submodule --cached status will also work as expected\n\n- Subcommands that currently do not take any arguments (init and update)\nhas a case which is introduced just to ensure that no argument is\ndeliberately sent as the first argument and also to serve the purpose of\nproviding a future extension point for its arguments.\n\n- Though ther was short and long version for quiet (-q or --quiet and\nbranch (-b or --branch) but there was no short version for cached. Thus\nit is now introduced (-c or --cached).\n\n- Added 3 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---\n git-submodule.sh |  158 +++++++++++++++++++++++++++++++++++++++---------------\n 1 files changed, 114 insertions(+), 44 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex ad9fe62..22e7e5f 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 [-q|--quiet] add [-b|--branch branch] <repository> [<path>]\n+# git-submodule [-q|--quiet] [status] [-c|--cached] [--] [<path>...]\n+# git-submodule [-q|--quiet] init [--] [<path>...]\n+# git-submodule [-q|--quiet] update [--] [<path>...]\n+USAGE='[-q|--quiet] [[[add [-b|--branch branch] <repo>]|[[[status [-c|--cached]]|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@@ -114,6 +119,17 @@ module_clone()\n \tdie \"Clone of '$url' into submodule path '$path' failed\"\n }\n \n+# Parses the branch name and exits if not present\n+parse_branch_name()\n+{\n+\tbranch=\"$1\"; \n+\tif test -z \"$branch\"\n+\tthen\n+\t\techo Branch name must me specified\t\n+\t\tusage\n+\tfi\n+}\n+\n #\n # Add a new submodule to the working tree, .gitmodules and the index\n #\n@@ -123,6 +139,16 @@ module_clone()\n #\n module_add()\n {\n+\tcase \"$1\" in\n+\t\t-b|--branch)\n+\t\t\tshift\n+\t\t\tparse_branch_name \"$@\" &&\n+\t\t\tshift\n+\t\t\t;;\n+\t\t-*)\n+\t\t\tusage\n+\t\t\t;;\n+\tesac\n \trepo=$1\n \tpath=$2\n \n@@ -176,6 +202,14 @@ module_add()\n #\n modules_init()\n {\n+\t# Added here to ensure that no argument is passed to be treated as\n+\t# parameter to the sub command. This will be used to parse any \n+\t# to the subcommand\n+\tcase \"$1\" in\n+\t\t-*)\n+\t\t\tusage\n+\t\t\t;;\n+\tesac\n \tgit ls-files --stage -- \"$@\" | grep -e '^160000 ' |\n \twhile read mode sha1 stage path\n \tdo\n@@ -209,6 +243,14 @@ modules_init()\n #\n modules_update()\n {\n+\t# Added here to ensure that no argument is passed to be treated as\n+\t# parameter to the sub command. This will be used to parse any \n+\t# to the subcommand\n+\tcase \"$1\" in\n+\t\t-*)\n+\t\t\tusage\n+\t\t\t;;\n+\tesac\n \tgit ls-files --stage -- \"$@\" | grep -e '^160000 ' |\n \twhile read mode sha1 stage path\n \tdo\n@@ -293,36 +335,69 @@ modules_list()\n \tdone\n }\n \n+# Delgates to modules_list after parsing its arguments\n+modules_status()\n+{\n+\tcase \"$1\" in\n+\t\t-c|--cached)\n+\t\t\tshift\n+\t\t\tcached=1\n+\t\t\t;;\n+\t\t-*)\n+\t\t\tusage\n+\t\t\t;;\n+\tesac\n+\t\"$MODULES_LIST\" \"$@\"\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+\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+# It is to be noted that pre-subcommand arguments are parsed\n+# just to have backward compatibility.\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\t;;\n-\tupdate)\n-\t\tupdate=1\n-\t\t;;\n-\tstatus)\n-\t\tstatus=1\n+\tinit|update|status)\n+\t\tcommand=\"modules_$1\"\n+\t\tshift\n+\t\tcheck_for_terminator \"$1\"\n+\t\tbreak\n \t\t;;\n \t-q|--quiet)\n \t\tquiet=1\n \t\t;;\n \t-b|--branch)\n-\t\tcase \"$2\" in\n-\t\t'')\n-\t\t\tusage\n-\t\t\t;;\n-\t\tesac\n-\t\tbranch=\"$2\"; shift\n+\t\tshift\n+\t\tparse_branch_name \"$@\"\n \t\t;;\n-\t--cached)\n+\t-c|--cached)\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 +410,25 @@ 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 then default command\n+# is - git submodule status\n+test -z \"$command\" && command=\"modules_status\"\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_status\"\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":"65367","messageId":"7vzlv7flb5.fsf@gitster.siamese.dyndns.org","threadId":"11614","inReplyTo":"1200280956-19920-1-git-send-email-imyousuf@gmail.com","subject":"Re: [PATCH] - git submodule subcommand parsing modified.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-15T10:13:50Z","receivedAt":"2008-01-15T10:13:50Z","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\n> \tgit-submodule add init update\n> as the command parser incorrectly made subcommand names reserve.\n> Thus refusing them to be used as parameters to subcommands. As a result it\n> was impossible to add a submodule whose (symbolic) name is \"init\" and that\n> resides at path \"update\" was refused. For more details the following case\n> can be considered -\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'd drop everything after \"For more details...\".  If you feel\n   that the part before \"For more details\" cannot be understood\n   without that thick and solid sample script, perhaps that\n   description needs to be rewritten to make it easier to\n   understand (personally I do not see it as hard to understand,\n   modulo grammatical errors).\n\n> This patch fixes this issue and allows it as well.\n\n - \"it\" in this sentence can easily be mistaken as referring to\n   \"this issue\".  I'd suggest dropping \"and allows...\".\n\n> - Status currently is implemented to show list only but later\n> implementation might change and list and status could coexists. Thus\n> status module is introduced. The module is also used to parse its\n> arguments\n\n - That is probably better in a separate patch, if the purpose\n   of the patch is about straightening out the command parser to\n   fix existing issues.  Generally it is a good idea to have\n   fixes and enhancement as separate patches.\n\n> - Subcommands will also parse their own commands; thus enabling command\n> specific arguments to be passed after the command. For example,\n> \tgit-submodule -q add -b master module_a\n> \tgit-submodule -q status -c\n> It is to be noted that -q or --quiet is specified before the subcommand\n> since it is for the submodule command in general rather than the\n> subcommand. It is mention worthy that backward compatibility exists and\n> thus commands like git submodule --cached status will also work as expected\n\n - I have a mild distaste against commands that expect the users\n   to intimately know what option is command wide and what\n   option is subcommand specific.  IOW it is not very nice to\n   accept \"git submodule -q status\" and reject \"git submodule\n   status -q\".\n\n> - Subcommands that currently do not take any arguments (init and update)\n> has a case which is introduced just to ensure that no argument is\n> deliberately sent as the first argument and also to serve the purpose of\n> providing a future extension point for its arguments.\n\n - As far as I understand what they do, they both do take\n   paths arguments to name specific modules, so the above\n   sentence is bogus.  Maybe you meant rejecting non-existent\n   options, and I'd agree with the intent if that is the case\n   (but your implementation around -- is bogus).\n\n> - Though ther was short and long version for quiet (-q or --quiet and\n> branch (-b or --branch) but there was no short version for cached. Thus\n> it is now introduced (-c or --cached).\n>\n> - Added 3 specific messages for usage error related to branch and cached\n>\n> - Simplified subcommand action invocation by simply invoking the action if\n> all conditions are fulfilled. Excepting for parsing command line arguments\n> case statements are avoided and instead more direct if statement is\n> introduced.\n> ---\n\n - Lacks sign-off.\n\n - The message felt very hard to read.  Perhaps it is just that\n   these unindented sentences in bullet-list, together with the\n   two displayed command line that are not separated from the\n   rest of the text with a blank line, hurts the eyes.\n\n - Perhaps the patch tries to do too many things.  For example,\n   introduction of -c does not have to belong here.  nor\n   \"status\" which currently is the same as \"list\".  Maybe that\n   is why the description needs to talk about too many things,\n   which in turn could be the reason why I find the above\n   message very hard to read.\n\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index ad9fe62..22e7e5f 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 [-q|--quiet] add [-b|--branch branch] <repository> [<path>]\n> +# git-submodule [-q|--quiet] [status] [-c|--cached] [--] [<path>...]\n> +# git-submodule [-q|--quiet] init [--] [<path>...]\n> +# git-submodule [-q|--quiet] update [--] [<path>...]\n> +USAGE='[-q|--quiet] [[[add [-b|--branch branch] <repo>]|[[[status [-c|--cached]]|init|update] [--]]]  [<path>...]]'\n>  OPTIONS_SPEC=\n>  . git-sh-setup\n>  require_work_tree\n>  \n> +MODULES_LIST='modules_list'\n> +\n\n - What's the purpose of this variable?  If you are planning to\n   change the default command by changing the value of this\n   variable, then the variable is not about MODULES_LIST, but\n   about DEFAULT_COMMAND or something, and should be named as\n   such.\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> @@ -114,6 +119,17 @@ module_clone()\n>  \tdie \"Clone of '$url' into submodule path '$path' failed\"\n>  }\n>  \n> +# Parses the branch name and exits if not present\n> +parse_branch_name()\n> +{\n> +\tbranch=\"$1\"; \n> +\tif test -z \"$branch\"\n> +\tthen\n> +\t\techo Branch name must me specified\t\n> +\t\tusage\n> +\tfi\n\n - Shouldn't the message go to stderr?\n\n - If we use \"usage\", that already tells the user -b is followed\n   by branch.  Is the extra message needed in the first place?\n   If this patch is about \"fixing\", perhaps you should not be\n   mixing this kind of unnecessary changes in it.\n\n> +}\n> +\n>  #\n>  # Add a new submodule to the working tree, .gitmodules and the index\n>  #\n> @@ -123,6 +139,16 @@ module_clone()\n>  #\n>  module_add()\n>  {\n> +\tcase \"$1\" in\n> +\t\t-b|--branch)\n> +\t\t\tshift\n> +\t\t\tparse_branch_name \"$@\" &&\n> +\t\t\tshift\n> +\t\t\t;;\n> +\t\t-*)\n> +\t\t\tusage\n> +\t\t\t;;\n> +\tesac\n\n - Style.  Please align case arms with \"case/esac\", like other\n   scripts (and the original version of this script) do.  I.e.\n\n\tcase \"$1\" in\n        -b|--branch)\n                ...\n\tesac\n\n   This comment applies to other parts of the patch as well.\n\n>  \trepo=$1\n>  \tpath=$2\n>  \n> @@ -176,6 +202,14 @@ module_add()\n>  #\n>  modules_init()\n>  {\n> +\t# Added here to ensure that no argument is passed to be treated as\n> +\t# parameter to the sub command. This will be used to parse any \n> +\t# to the subcommand\n> +\tcase \"$1\" in\n> +\t\t-*)\n> +\t\t\tusage\n> +\t\t\t;;\n> +\tesac\n\n - If I understand correctly, \"git submodule init\" takes paths\n   to submodules as arguments.  Are you disallowing paths that\n   begin with '-', even though the body of the command (ls-files\n   piped to while loop) is written in such a way that is\n   perfectly capable of handling such a path?\n\n>  \tgit ls-files --stage -- \"$@\" | grep -e '^160000 ' |\n>  \twhile read mode sha1 stage path\n>  \tdo\n> @@ -209,6 +243,14 @@ modules_init()\n>  #\n>  modules_update()\n>  {\n> +\t# Added here to ensure that no argument is passed to be treated as\n> +\t# parameter to the sub command. This will be used to parse any \n> +\t# to the subcommand\n> +\tcase \"$1\" in\n> +\t\t-*)\n> +\t\t\tusage\n> +\t\t\t;;\n> +\tesac\n\n - The same comment as modules_init() above applies here.\n\n>  \tgit ls-files --stage -- \"$@\" | grep -e '^160000 ' |\n>  \twhile read mode sha1 stage path\n>  \tdo\n> @@ -293,36 +335,69 @@ modules_list()\n>  \tdone\n>  }\n>  \n> +# Delgates to modules_list after parsing its arguments\n\n - That's \"delegates\", but typically when we write a sentence\n   without the subject like this in the comment, we use the\n   imperative mood, so \"Delegate to modules_list after ...\"\n   would be more appropriate.\n\n> +modules_status()\n> +{\n> +\tcase \"$1\" in\n> +\t\t-c|--cached)\n> +\t\t\tshift\n> +\t\t\tcached=1\n> +\t\t\t;;\n> +\t\t-*)\n> +\t\t\tusage\n> +\t\t\t;;\n> +\tesac\n> +\t\"$MODULES_LIST\" \"$@\"\n> +}\n\n - The same comment as modules_init() above applies here.\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 - That 'test -n \"$1\" && test \"$1\" = \"--\"' feels stupid; if \"$1\"\n   is equal to \"--\", it certainly will be -n (i.e. not empty).\n   Perhaps 'test $# -ge 1 && test \"$1\" = \"--\"' or even just\n   'test \"$1\" = \"--\"'.\n\n - I do not think the 'shift' does anything useful.  It does not\n   shift the positional parameters of the caller of this shell\n   function, and would be a noop from the caller's point of\n   view.  It only shifts the positional parameters inside this\n   function (study e.g. \"2.9.5 Function Definition Command\",\n   http://www.opengroup.org/onlinepubs/000095399/utilities/xcu_chap02.html).\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\n - That's a valid justification but I think it is enough to say\n   what it does (i.e. \"Arguments after the subcommand name are\n   given to the subcommand\").  How you arrived to that design\n   decision (i.e. your justification based on the synopsis) does\n   not belong here.  It could however be part of the commit log\n   message.\n\n - I do not agree with the (attempted) stripping of -- you talk\n   about here (that is done in the loop below).\n\n> +# It is to be noted that pre-subcommand arguments are parsed\n> +# just to have backward compatibility.\n\n - Because we might want to deprecate and remove forms like \"-b\n   branch add\" later, this is a good comment to have here.  It\n   makes it clear that these oddballs are purely for backward\n   compatibility.\n\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\t;;\n> -\tupdate)\n> -\t\tupdate=1\n> -\t\t;;\n> -\tstatus)\n> -\t\tstatus=1\n> +\tinit|update|status)\n> +\t\tcommand=\"modules_$1\"\n> +\t\tshift\n> +\t\tcheck_for_terminator \"$1\"\n> +\t\tbreak\n>  \t\t;;\n\n - Aside from my earlier comment that the code would become\n   simpler if you consistently renamed the shell functions to\n   module_$foo (or cmd_$foo) so that the dispatcher can follow a\n   simple rule \"subcommand $foo is handled by shell function\n   cmd_$foo\", which you seem to have ignored, and also aside\n   from that your check_for_terminator does not do what you seem\n   to have intended (see above), I think this handling of -- is\n   wrong.  By stripping -- here, you are making the following\n   two behave exactly the same:\n\n   $ git submodule update -- $other_args\n   $ git submodule update    $other_args\n\n   The whole point of -- is so that you can tell the command\n   that the argument at the beginning of $other_args that\n   happens to begin with a dash is _not_ an option but is a\n   literal path (or whatever).  Think of the case in which you\n   had '-foo' and 'bar' in place of $other_args above.  The\n   first one tells the command to update two modules ('-foo' and\n   'bar'), the second one tells the command to update 'bar'\n   module but the update operation needs to be done with -foo\n   option (whatever that option means to 'update' command).\n\n   By checking and shifting -- out, you are making it impossible\n   for the implementation of the command (i.e. your\n   \"modules_update\") to tell which case it is dealing with.\n\n>  \t-q|--quiet)\n>  \t\tquiet=1\n>  \t\t;;\n>  \t-b|--branch)\n> -\t\tcase \"$2\" in\n> -\t\t'')\n> -\t\t\tusage\n> -\t\t\t;;\n> -\t\tesac\n> -\t\tbranch=\"$2\"; shift\n> +\t\tshift\n> +\t\tparse_branch_name \"$@\"\n>  \t\t;;\n> -\t--cached)\n> +\t-c|--cached)\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 +410,25 @@ 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 then default command\n> +# is - git submodule status\n> +test -z \"$command\" && command=\"modules_status\"\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_status\"\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> -- \n> 1.5.3.7\n"},{"id":"65371","messageId":"7vy7are3qo.fsf_-_@gitster.siamese.dyndns.org","threadId":"11614","inReplyTo":"7vzlv7flb5.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 1/3] git-submodule: rename shell functions for consistency","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-15T11:18:39Z","receivedAt":"2008-01-15T11:18:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This renames the shell functions used in git-submodule that\nimplement top-level subcommands.  The rule is that the\nsubcommand $foo is implemented by cmd_$foo function.\n\nA noteworthy change is that modules_list() is now known as\ncmd_status().  There is no \"submodule list\" command.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * We could probably do something like this.  This first part is\n   about making the command dispatcher maintainable.\n\n   Note that I haven't seriously tested this series.  This and\n   the next one are primarily to illustrate what I think the fix\n   you are trying should look like.\n\n git-submodule.sh |   20 ++++++++++----------\n 1 files changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex ad9fe62..3c104e3 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -86,9 +86,9 @@ module_name()\n #\n # Clone a submodule\n #\n-# Prior to calling, modules_update checks that a possibly existing\n+# Prior to calling, cmd_update checks that a possibly existing\n # path is not a git repository.\n-# Likewise, module_add checks that path does not exist at all,\n+# Likewise, cmd_add checks that path does not exist at all,\n # since it is the location of a new submodule.\n #\n module_clone()\n@@ -121,7 +121,7 @@ module_clone()\n #\n # optional branch is stored in global branch variable\n #\n-module_add()\n+cmd_add()\n {\n \trepo=$1\n \tpath=$2\n@@ -174,7 +174,7 @@ module_add()\n #\n # $@ = requested paths (default to all)\n #\n-modules_init()\n+cmd_init()\n {\n \tgit ls-files --stage -- \"$@\" | grep -e '^160000 ' |\n \twhile read mode sha1 stage path\n@@ -207,7 +207,7 @@ modules_init()\n #\n # $@ = requested paths (default to all)\n #\n-modules_update()\n+cmd_update()\n {\n \tgit ls-files --stage -- \"$@\" | grep -e '^160000 ' |\n \twhile read mode sha1 stage path\n@@ -266,7 +266,7 @@ set_name_rev () {\n #\n # $@ = requested paths (default to all)\n #\n-modules_list()\n+cmd_status()\n {\n \tgit ls-files --stage -- \"$@\" | grep -e '^160000 ' |\n \twhile read mode sha1 stage path\n@@ -347,16 +347,16 @@ esac\n \n case \"$add,$init,$update,$status,$cached\" in\n 1,,,,)\n-\tmodule_add \"$@\"\n+\tcmd_add \"$@\"\n \t;;\n ,1,,,)\n-\tmodules_init \"$@\"\n+\tcmd_init \"$@\"\n \t;;\n ,,1,,)\n-\tmodules_update \"$@\"\n+\tcmd_update \"$@\"\n \t;;\n ,,,*,*)\n-\tmodules_list \"$@\"\n+\tcmd_status \"$@\"\n \t;;\n *)\n \tusage\n-- \n1.5.4.rc3.11.g4e67\n"},{"id":"65372","messageId":"7vve5ve3ov.fsf_-_@gitster.siamese.dyndns.org","threadId":"11614","inReplyTo":"7vzlv7flb5.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 2/3] git-submodule: fix subcommand parser","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-15T11:19:44Z","receivedAt":"2008-01-15T11:19:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The subcommand parser of \"git submodule\" made its subcommand\nnames reserved words.  As a consequence, a command like this:\n\n    $ git submodule add init update\n\nwhich is meant to add a submodule called 'init' at path 'update'\nwas misinterpreted as a request to invoke more than one mutually\nincompatible subcommands and incorrectly rejected.\n\nThis patch fixes the issue by stopping the subcommand parsing at\nthe first subcommand word, to allow the sample command line\nabove to work as expected.\n\nIt also introduces the usual -- option disambiguator, so that a\nsubmodule at path '-foo' can be updated with\n\n    $ git submodule update -- -foo\n\nwithout triggering an \"unrecognized option -foo\" error.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * And this is the second part to fix the real issue.\n\n git-submodule.sh |  157 ++++++++++++++++++++++++++++++++++++++++--------------\n 1 files changed, 116 insertions(+), 41 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 3c104e3..a6aaf40 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -9,11 +9,8 @@ OPTIONS_SPEC=\n . git-sh-setup\n require_work_tree\n \n-add=\n+command=\n branch=\n-init=\n-update=\n-status=\n quiet=\n cached=\n \n@@ -123,6 +120,32 @@ module_clone()\n #\n cmd_add()\n {\n+\t# parse $args after \"submodule ... add\".\n+\twhile test $# -ne 0\n+\tdo\n+\t\tcase \"$1\" in\n+\t\t-b | --branch)\n+\t\t\tcase \"$2\" in '') usage ;; esac\n+\t\t\tbranch=$2\n+\t\t\tshift\n+\t\t\t;;\n+\t\t-q|--quiet)\n+\t\t\tquiet=1\n+\t\t\t;;\n+\t\t--)\n+\t\t\tshift\n+\t\t\tbreak\n+\t\t\t;;\n+\t\t-*)\n+\t\t\tusage\n+\t\t\t;;\n+\t\t*)\n+\t\t\tbreak\n+\t\t\t;;\n+\t\tesac\n+\t\tshift\n+\tdone\n+\n \trepo=$1\n \tpath=$2\n \n@@ -176,6 +199,27 @@ cmd_add()\n #\n cmd_init()\n {\n+\t# parse $args after \"submodule ... init\".\n+\twhile test $# -ne 0\n+\tdo\n+\t\tcase \"$1\" in\n+\t\t-q|--quiet)\n+\t\t\tquiet=1\n+\t\t\t;;\n+\t\t--)\n+\t\t\tshift\n+\t\t\tbreak\n+\t\t\t;;\n+\t\t-*)\n+\t\t\tusage\n+\t\t\t;;\n+\t\t*)\n+\t\t\tbreak\n+\t\t\t;;\n+\t\tesac\n+\t\tshift\n+\tdone\n+\n \tgit ls-files --stage -- \"$@\" | grep -e '^160000 ' |\n \twhile read mode sha1 stage path\n \tdo\n@@ -209,6 +253,27 @@ cmd_init()\n #\n cmd_update()\n {\n+\t# parse $args after \"submodule ... update\".\n+\twhile test $# -ne 0\n+\tdo\n+\t\tcase \"$1\" in\n+\t\t-q|--quiet)\n+\t\t\tquiet=1\n+\t\t\t;;\n+\t\t--)\n+\t\t\tshift\n+\t\t\tbreak\n+\t\t\t;;\n+\t\t-*)\n+\t\t\tusage\n+\t\t\t;;\n+\t\t*)\n+\t\t\tbreak\n+\t\t\t;;\n+\t\tesac\n+\t\tshift\n+\tdone\n+\n \tgit ls-files --stage -- \"$@\" | grep -e '^160000 ' |\n \twhile read mode sha1 stage path\n \tdo\n@@ -268,6 +333,30 @@ set_name_rev () {\n #\n cmd_status()\n {\n+\t# parse $args after \"submodule ... status\".\n+\twhile test $# -ne 0\n+\tdo\n+\t\tcase \"$1\" in\n+\t\t-q|--quiet)\n+\t\t\tquiet=1\n+\t\t\t;;\n+\t\t--cached)\n+\t\t\tcached=1\n+\t\t\t;;\n+\t\t--)\n+\t\t\tshift\n+\t\t\tbreak\n+\t\t\t;;\n+\t\t-*)\n+\t\t\tusage\n+\t\t\t;;\n+\t\t*)\n+\t\t\tbreak\n+\t\t\t;;\n+\t\tesac\n+\t\tshift\n+\tdone\n+\n \tgit ls-files --stage -- \"$@\" | grep -e '^160000 ' |\n \twhile read mode sha1 stage path\n \tdo\n@@ -293,20 +382,17 @@ cmd_status()\n \tdone\n }\n \n-while test $# != 0\n+# This loop parses the command line arguments to find the\n+# subcommand name to dispatch.  Parsing of the subcommand specific\n+# options are primarily done by the subcommand implementations.\n+# Subcommand specific options such as --branch and --cached are\n+# parsed here as well, for backward compatibility.\n+\n+while test $# != 0 && test -z \"$command\"\n do\n \tcase \"$1\" in\n-\tadd)\n-\t\tadd=1\n-\t\t;;\n-\tinit)\n-\t\tinit=1\n-\t\t;;\n-\tupdate)\n-\t\tupdate=1\n-\t\t;;\n-\tstatus)\n-\t\tstatus=1\n+\tadd | init | update | status)\n+\t\tcommand=$1\n \t\t;;\n \t-q|--quiet)\n \t\tquiet=1\n@@ -335,30 +421,19 @@ do\n \tshift\n done\n \n-case \"$add,$branch\" in\n-1,*)\n-\t;;\n-,)\n-\t;;\n-,*)\n+# No command word defaults to \"status\"\n+test -n \"$command\" || command=status\n+\n+# \"-b branch\" is accepted only by \"add\"\n+if test -n \"$branch\" && test \"$command\" != add\n+then\n \tusage\n-\t;;\n-esac\n-\n-case \"$add,$init,$update,$status,$cached\" in\n-1,,,,)\n-\tcmd_add \"$@\"\n-\t;;\n-,1,,,)\n-\tcmd_init \"$@\"\n-\t;;\n-,,1,,)\n-\tcmd_update \"$@\"\n-\t;;\n-,,,*,*)\n-\tcmd_status \"$@\"\n-\t;;\n-*)\n+fi\n+\n+# \"--cached\" is accepted only by \"status\"\n+if test -n \"$cached\" && test \"$command\" != status\n+then\n \tusage\n-\t;;\n-esac\n+fi\n+\n+\"cmd_$command\" \"$@\"\n-- \n1.5.4.rc3.11.g4e67\n"},{"id":"65373","messageId":"7vtzlfe3o5.fsf_-_@gitster.siamese.dyndns.org","threadId":"11614","inReplyTo":"7vzlv7flb5.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 3/3] git-submodule: add test for the subcommand parser fix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-15T11:20:10Z","receivedAt":"2008-01-15T11:20:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This modifies the existing t7400 test to use 'init' as the\npathname that a submodule is bound to.  Without the earlier\nsubcommand parser fix, this fails.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t7400-submodule-basic.sh |   56 ++++++++++++++++++++++----------------------\n 1 files changed, 28 insertions(+), 28 deletions(-)\n\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex 4fe3a41..2ef85a8 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -13,11 +13,11 @@ subcommands of git-submodule.\n \n #\n # Test setup:\n-#  -create a repository in directory lib\n+#  -create a repository in directory init\n #  -add a couple of files\n-#  -add directory lib to 'superproject', this creates a DIRLINK entry\n+#  -add directory init to 'superproject', this creates a DIRLINK entry\n #  -add a couple of regular files to enable testing of submodule filtering\n-#  -mv lib subrepo\n+#  -mv init subrepo\n #  -add an entry to .gitmodules for submodule 'example'\n #\n test_expect_success 'Prepare submodule testing' '\n@@ -25,8 +25,8 @@ test_expect_success 'Prepare submodule testing' '\n \tgit-add t &&\n \tgit-commit -m \"initial commit\" &&\n \tgit branch initial HEAD &&\n-\tmkdir lib &&\n-\tcd lib &&\n+\tmkdir init &&\n+\tcd init &&\n \tgit init &&\n \techo a >a &&\n \tgit add a &&\n@@ -41,10 +41,10 @@ test_expect_success 'Prepare submodule testing' '\n \tcd .. &&\n \techo a >a &&\n \techo z >z &&\n-\tgit add a lib z &&\n+\tgit add a init z &&\n \tgit-commit -m \"super commit 1\" &&\n-\tmv lib .subrepo &&\n-\tGIT_CONFIG=.gitmodules git config submodule.example.url git://example.com/lib.git\n+\tmv init .subrepo &&\n+\tGIT_CONFIG=.gitmodules git config submodule.example.url git://example.com/init.git\n '\n \n test_expect_success 'status should fail for unmapped paths' '\n@@ -52,7 +52,7 @@ test_expect_success 'status should fail for unmapped paths' '\n \tthen\n \t\techo \"[OOPS] submodule status succeeded\"\n \t\tfalse\n-\telif ! GIT_CONFIG=.gitmodules git config submodule.example.path lib\n+\telif ! GIT_CONFIG=.gitmodules git config submodule.example.path init\n \tthen\n \t\techo \"[OOPS] git config failed to update .gitmodules\"\n \t\tfalse\n@@ -71,7 +71,7 @@ test_expect_success 'status should initially be \"missing\"' '\n test_expect_success 'init should register submodule url in .git/config' '\n \tgit-submodule init &&\n \turl=$(git config submodule.example.url) &&\n-\tif test \"$url\" != \"git://example.com/lib.git\"\n+\tif test \"$url\" != \"git://example.com/init.git\"\n \tthen\n \t\techo \"[OOPS] init succeeded but submodule url is wrong\"\n \t\tfalse\n@@ -83,41 +83,41 @@ test_expect_success 'init should register submodule url in .git/config' '\n '\n \n test_expect_success 'update should fail when path is used by a file' '\n-\techo \"hello\" >lib &&\n+\techo \"hello\" >init &&\n \tif git-submodule update\n \tthen\n \t\techo \"[OOPS] update should have failed\"\n \t\tfalse\n-\telif test \"$(cat lib)\" != \"hello\"\n+\telif test \"$(cat init)\" != \"hello\"\n \tthen\n-\t\techo \"[OOPS] update failed but lib file was molested\"\n+\t\techo \"[OOPS] update failed but init file was molested\"\n \t\tfalse\n \telse\n-\t\trm lib\n+\t\trm init\n \tfi\n '\n \n test_expect_success 'update should fail when path is used by a nonempty directory' '\n-\tmkdir lib &&\n-\techo \"hello\" >lib/a &&\n+\tmkdir init &&\n+\techo \"hello\" >init/a &&\n \tif git-submodule update\n \tthen\n \t\techo \"[OOPS] update should have failed\"\n \t\tfalse\n-\telif test \"$(cat lib/a)\" != \"hello\"\n+\telif test \"$(cat init/a)\" != \"hello\"\n \tthen\n-\t\techo \"[OOPS] update failed but lib/a was molested\"\n+\t\techo \"[OOPS] update failed but init/a was molested\"\n \t\tfalse\n \telse\n-\t\trm lib/a\n+\t\trm init/a\n \tfi\n '\n \n test_expect_success 'update should work when path is an empty dir' '\n-\trm -rf lib &&\n-\tmkdir lib &&\n+\trm -rf init &&\n+\tmkdir init &&\n \tgit-submodule update &&\n-\thead=$(cd lib && git rev-parse HEAD) &&\n+\thead=$(cd init && git rev-parse HEAD) &&\n \tif test -z \"$head\"\n \tthen\n \t\techo \"[OOPS] Failed to obtain submodule head\"\n@@ -134,7 +134,7 @@ test_expect_success 'status should be \"up-to-date\" after update' '\n '\n \n test_expect_success 'status should be \"modified\" after submodule commit' '\n-\tcd lib &&\n+\tcd init &&\n \techo b >b &&\n \tgit add b &&\n \tgit-commit -m \"submodule commit 2\" &&\n@@ -157,8 +157,8 @@ test_expect_success 'git diff should report the SHA1 of the new submodule commit\n '\n \n test_expect_success 'update should checkout rev1' '\n-\tgit-submodule update &&\n-\thead=$(cd lib && git rev-parse HEAD) &&\n+\tgit-submodule update init &&\n+\thead=$(cd init && git rev-parse HEAD) &&\n \tif test -z \"$head\"\n \tthen\n \t\techo \"[OOPS] submodule git rev-parse returned nothing\"\n@@ -182,13 +182,13 @@ test_expect_success 'checkout superproject with subproject already present' '\n test_expect_success 'apply submodule diff' '\n \tgit branch second &&\n \t(\n-\t\tcd lib &&\n+\t\tcd init &&\n \t\techo s >s &&\n \t\tgit add s &&\n \t\tgit commit -m \"change subproject\"\n \t) &&\n-\tgit update-index --add lib &&\n-\tgit-commit -m \"change lib\" &&\n+\tgit update-index --add init &&\n+\tgit-commit -m \"change init\" &&\n \tgit-format-patch -1 --stdout >P.diff &&\n \tgit checkout second &&\n \tgit apply --index P.diff &&\n-- \n1.5.4.rc3.11.g4e67\n"},{"id":"65466","messageId":"7bfdc29a0801151826u2218f825ga8100b1cc9fa8b2@mail.gmail.com","threadId":"11614","inReplyTo":"7vy7are3qo.fsf_-_@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/3] git-submodule: rename shell functions for consistency","fromName":"Imran M Yousuf","fromEmail":"imyousuf@gmail.com","sentAt":"2008-01-16T02:26:29Z","receivedAt":"2008-01-16T02:26:29Z","isPatch":true,"sender":{"key":"imyousuf@gmail.com","avatar":"https://gravatar.com/avatar/fda3c870262849d03c7b9c4d288842e128d6d80769fa7bc2d22731b7597928be?d=mp&s=160"},"body":"Thanks Junio for showing how it should be done. Due to some\npre-scheduled appointment I was unavailable yesterday evening and thus\nwas neither able to reply nor resubmit the changes.\n\nOn Jan 15, 2008 5:18 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> This renames the shell functions used in git-submodule that\n> implement top-level subcommands.  The rule is that the\n> subcommand $foo is implemented by cmd_$foo function.\n>\n> A noteworthy change is that modules_list() is now known as\n> cmd_status().  There is no \"submodule list\" command.\n>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>\n>  * We could probably do something like this.  This first part is\n>    about making the command dispatcher maintainable.\n>\n>    Note that I haven't seriously tested this series.  This and\n>    the next one are primarily to illustrate what I think the fix\n>    you are trying should look like.\n>\n>  git-submodule.sh |   20 ++++++++++----------\n>  1 files changed, 10 insertions(+), 10 deletions(-)\n>\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index ad9fe62..3c104e3 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -86,9 +86,9 @@ module_name()\n>  #\n>  # Clone a submodule\n>  #\n> -# Prior to calling, modules_update checks that a possibly existing\n> +# Prior to calling, cmd_update checks that a possibly existing\n>  # path is not a git repository.\n> -# Likewise, module_add checks that path does not exist at all,\n> +# Likewise, cmd_add checks that path does not exist at all,\n>  # since it is the location of a new submodule.\n>  #\n>  module_clone()\n> @@ -121,7 +121,7 @@ module_clone()\n>  #\n>  # optional branch is stored in global branch variable\n>  #\n> -module_add()\n> +cmd_add()\n\nAfter reading your reply I was about to suggest renaming module to cmd\nbut you have done it before I could propose or submit the patch.\n\n>  {\n>         repo=$1\n>         path=$2\n> @@ -174,7 +174,7 @@ module_add()\n>  #\n>  # $@ = requested paths (default to all)\n>  #\n> -modules_init()\n> +cmd_init()\n>  {\n>         git ls-files --stage -- \"$@\" | grep -e '^160000 ' |\n>         while read mode sha1 stage path\n> @@ -207,7 +207,7 @@ modules_init()\n>  #\n>  # $@ = requested paths (default to all)\n>  #\n> -modules_update()\n> +cmd_update()\n>  {\n>         git ls-files --stage -- \"$@\" | grep -e '^160000 ' |\n>         while read mode sha1 stage path\n> @@ -266,7 +266,7 @@ set_name_rev () {\n>  #\n>  # $@ = requested paths (default to all)\n>  #\n> -modules_list()\n> +cmd_status()\n>  {\n>         git ls-files --stage -- \"$@\" | grep -e '^160000 ' |\n>         while read mode sha1 stage path\n> @@ -347,16 +347,16 @@ esac\n>\n>  case \"$add,$init,$update,$status,$cached\" in\n>  1,,,,)\n> -       module_add \"$@\"\n> +       cmd_add \"$@\"\n>         ;;\n>  ,1,,,)\n> -       modules_init \"$@\"\n> +       cmd_init \"$@\"\n>         ;;\n>  ,,1,,)\n> -       modules_update \"$@\"\n> +       cmd_update \"$@\"\n>         ;;\n>  ,,,*,*)\n> -       modules_list \"$@\"\n> +       cmd_status \"$@\"\n>         ;;\n>  *)\n>         usage\n> --\n> 1.5.4.rc3.11.g4e67\n>\n>\n\n\n\n-- \nImran M Yousuf\n"},{"id":"65593","messageId":"7vbq7lpm78.fsf@gitster.siamese.dyndns.org","threadId":"11614","inReplyTo":"7bfdc29a0801151826u2218f825ga8100b1cc9fa8b2@mail.gmail.com","subject":"Re: [PATCH 1/3] git-submodule: rename shell functions for consistency","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-16T20:08:59Z","receivedAt":"2008-01-16T20:08:59Z","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> Thanks Junio for showing how it should be done. Due to some\n> pre-scheduled appointment I was unavailable yesterday evening and thus\n> was neither able to reply nor resubmit the changes.\n\nWell, I did not show how it _should_ be done.  That series was\nmerely an illustration of how I _think_ it should look like.  I\ndid not test it, I do not know if it introduced new bugs, and\nmost importantly I do not know if it fulfills what you intended\nto achieve with your patch.\n\nIn other words, I just tried to turn the table around.  Instead\nof me and others commenting on your patch saying \"I do not like\nthis\" piecemeal, now you have something you can comment on.  You\ncan say the whole range of things from \"I tested this and it is\nwhat I want\", \"I like the general concept but I found this and\nthat bug and here is a fix\", to \"This is much worse than what I\nproposed and here is why.\"\n"}]}