{"thread":{"id":"62611","subject":"[PATCH 0/3] git-submodule.sh: improve parsing of options","startedAt":"2024-12-07T13:52:47Z","lastAt":"2024-12-11T06:34:01Z","messageCount":43,"participants":["Roy Eldar","Junio C Hamano","Eric Sunshine","Roy E","Đoàn Trần Công Danh"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"508819","messageId":"20241207135201.2536-1-royeldar0@gmail.com","threadId":"62611","inReplyTo":null,"subject":"[PATCH 0/3] git-submodule.sh: improve parsing of options","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-07T13:51:58Z","receivedAt":"2024-12-07T13:52:47Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"When we run \"git submodule\", the script parses the various options and\nthen invokes \"git-submodule--helper\". Unlike most builtin git commands\nwhich parse short/long options using parse-options.c, the parsing of\narguments is completely done within git-submodule.sh; therefore, there\nare some inconsistencies with the rest of the commands, in particular\nthe parsing of option arguments given to various options.\n\nImprove the handling of option arguments for both long & short options;\nfor example, passing flags such as \"--branch=master\" or \"-j8\" now works.\n\nRoy Eldar (3):\n git-submodule.sh: make some variables boolean\n git-submodule.sh: improve parsing of some long options\n git-submodule.sh: improve parsing of short options\n\n git-submodule.sh | 32 +++++++++++++++++++++++++++-----\n 1 file changed, 27 insertions(+), 5 deletions(-)\n\n-- \n2.30.2\n\n"},{"id":"508820","messageId":"20241207135201.2536-2-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241207135201.2536-1-royeldar0@gmail.com","subject":"[PATCH 1/3] git-submodule.sh: make some variables boolean","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-07T13:51:59Z","receivedAt":"2024-12-07T13:52:59Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"When git-submodule.sh parses various options and switchs, it sets some\nvariables to values; in particular, every switch that is passed causes a\ncorresponding variable to be set to 1, which then affects the options\ngiven to git-submodule--helper.\n\nThere are some variables are assigned \"$1\", although there is no reason\nfor it; this was actually noticed in 757d092 for the \"$cached\" variable.\n\nMake some variables boolean, in order to increase consistency throught\nthe script and reduce possible confusion.\n\nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 03c5a220a2..107011f613 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -78,7 +78,7 @@ cmd_add()\n \t\t\tshift\n \t\t\t;;\n \t\t-f | --force)\n-\t\t\tforce=$1\n+\t\t\tforce=1\n \t\t\t;;\n \t\t-q|--quiet)\n \t\t\tquiet=1\n@@ -231,7 +231,7 @@ cmd_deinit()\n \tdo\n \t\tcase \"$1\" in\n \t\t-f|--force)\n-\t\t\tforce=$1\n+\t\t\tforce=1\n \t\t\t;;\n \t\t-q|--quiet)\n \t\t\tquiet=1\n@@ -294,7 +294,7 @@ cmd_update()\n \t\t\tnofetch=1\n \t\t\t;;\n \t\t-f|--force)\n-\t\t\tforce=$1\n+\t\t\tforce=1\n \t\t\t;;\n \t\t-r|--rebase)\n \t\t\trebase=1\n@@ -500,10 +500,10 @@ cmd_summary() {\n \t\t\tcached=1\n \t\t\t;;\n \t\t--files)\n-\t\t\tfiles=\"$1\"\n+\t\t\tfiles=1\n \t\t\t;;\n \t\t--for-status)\n-\t\t\tfor_status=\"$1\"\n+\t\t\tfor_status=1\n \t\t\t;;\n \t\t-n|--summary-limit)\n \t\t\tsummary_limit=\"$2\"\n-- \n2.30.2\n\n"},{"id":"508821","messageId":"20241207135201.2536-3-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241207135201.2536-1-royeldar0@gmail.com","subject":"[PATCH 2/3] git-submodule.sh: improve parsing of some long options","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-07T13:52:00Z","receivedAt":"2024-12-07T13:53:01Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"Some command-line options have a long form which takes an argument. In\nthis case, the argument can be given right after `='; for example,\n\"--depth\" takes a numerical argument, which can be given as \"--depth=X\".\n\nSupport the case where the argument is given right after `=' for all\nlong options, in order to improve consistency throughout the script.\n\nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 9 +++++++++\n 1 file changed, 9 insertions(+)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 107011f613..a47d2a89f3 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -77,6 +77,9 @@ cmd_add()\n \t\t\tbranch=$2\n \t\t\tshift\n \t\t\t;;\n+\t\t--branch=*)\n+\t\t\tbranch=\"${1#--branch=}\"\n+\t\t\t;;\n \t\t-f | --force)\n \t\t\tforce=1\n \t\t\t;;\n@@ -110,6 +113,9 @@ cmd_add()\n \t\t\tcustom_name=$2\n \t\t\tshift\n \t\t\t;;\n+\t\t--name=*)\n+\t\t\tcustom_name=\"${1#--name=}\"\n+\t\t\t;;\n \t\t--depth)\n \t\t\tcase \"$2\" in '') usage ;; esac\n \t\t\tdepth=\"--depth=$2\"\n@@ -425,6 +431,9 @@ cmd_set_branch() {\n \t\t\tbranch=$2\n \t\t\tshift\n \t\t\t;;\n+\t\t--branch=*)\n+\t\t\tbranch=\"${1#--branch=}\"\n+\t\t\t;;\n \t\t--)\n \t\t\tshift\n \t\t\tbreak\n-- \n2.30.2\n\n"},{"id":"508822","messageId":"20241207135201.2536-4-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241207135201.2536-1-royeldar0@gmail.com","subject":"[PATCH 3/3] git-submodule.sh: improve parsing of short options","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-07T13:52:01Z","receivedAt":"2024-12-07T13:53:03Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"Some command-line options have a short form which takes an argument; for\nexample, \"--jobs\" has the form \"-j\", and it takes a numerical argument.\n\nWhen parsing short options, support the case where there is no space\nbetween the flag and the option argument, in order to improve\nconsistency with the rest of the builtin git commands.\n\nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 13 +++++++++++++\n 1 file changed, 13 insertions(+)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex a47d2a89f3..fc85458fb1 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -77,6 +77,9 @@ cmd_add()\n \t\t\tbranch=$2\n \t\t\tshift\n \t\t\t;;\n+\t\t-b*)\n+\t\t\tbranch=\"${1#-b}\"\n+\t\t\t;;\n \t\t--branch=*)\n \t\t\tbranch=\"${1#--branch=}\"\n \t\t\t;;\n@@ -352,6 +355,9 @@ cmd_update()\n \t\t\tjobs=\"--jobs=$2\"\n \t\t\tshift\n \t\t\t;;\n+\t\t-j*)\n+\t\t\tjobs=\"--jobs=${1#-j}\"\n+\t\t\t;;\n \t\t--jobs=*)\n \t\t\tjobs=$1\n \t\t\t;;\n@@ -431,6 +437,9 @@ cmd_set_branch() {\n \t\t\tbranch=$2\n \t\t\tshift\n \t\t\t;;\n+\t\t-b*)\n+\t\t\tbranch=\"${1#-b}\"\n+\t\t\t;;\n \t\t--branch=*)\n \t\t\tbranch=\"${1#--branch=}\"\n \t\t\t;;\n@@ -519,6 +528,10 @@ cmd_summary() {\n \t\t\tisnumber \"$summary_limit\" || usage\n \t\t\tshift\n \t\t\t;;\n+\t\t-n*)\n+\t\t\tsummary_limit=\"${1#-n}\"\n+\t\t\tisnumber \"$summary_limit\" || usage\n+\t\t\t;;\n \t\t--summary-limit=*)\n \t\t\tsummary_limit=\"${1#--summary-limit=}\"\n \t\t\tisnumber \"$summary_limit\" || usage\n-- \n2.30.2\n\n"},{"id":"508825","messageId":"xmqqzfl7rrdl.fsf@gitster.g","threadId":"62611","inReplyTo":"20241207135201.2536-2-royeldar0@gmail.com","subject":"Re: [PATCH 1/3] git-submodule.sh: make some variables boolean","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-07T23:43:34Z","receivedAt":"2024-12-07T23:43:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Roy Eldar writes:\n\n> When git-submodule.sh parses various options and switchs, it sets some\n> variables to values; in particular, every switch that is passed causes a\n> corresponding variable to be set to 1, which then affects the options\n> given to git-submodule--helper.\n>\n> There are some variables are assigned \"$1\", although there is no reason\n> for it; this was actually noticed in 757d092 for the \"$cached\" variable.\n\nWearing devil's advocate hat on.\n\nThis can cut both ways. If you are doing a thin wrapper front-end,\nwhen calling into the underlying machinery that has its own option\nparser, it is often easier to write and read\n\n submodule--helper $force $cached\n\ninstead of\n\n submodule--helper ${force:+--force} ${cached:+--cached}\n\nIn addition, when debugging such a script by running it under \"sh -x\",\na typical construct like\n\n\tif test \"$force\" = 1\n\tthen\n\t\t... do the force thing ...\n\nwould appear in the \"-x\" trace as\n\n\t+test 1 = 1\n\t+... do the force thing ...\n\nif you force yourself to use \"1\", but \n\n\tif test \"$force\" = \"--force\"\n\tthen\n\t\t... do the force thing ...\n\nwould show either\n\n\t+test --force = --force\n\nor\n\n\t+test \"\" = --force\n\nand the latter is especially useful when you are wondering why the\n\"--force\" you gave from the command line is not taking effect.\n\nHaving said all that, as long as the use of these switch variables\nare done consistently (e.g., consistently set effective ones to \"1\",\nand consistently set ones that are off to \"\"), we are OK.\n\nThanks.\n"},{"id":"508826","messageId":"xmqqr06jrqiv.fsf@gitster.g","threadId":"62611","inReplyTo":"20241207135201.2536-4-royeldar0@gmail.com","subject":"Re: [PATCH 3/3] git-submodule.sh: improve parsing of short options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-08T00:02:00Z","receivedAt":"2024-12-08T00:02:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Roy Eldar writes:\n\n> Some command-line options have a short form which takes an argument; for\n> example, \"--jobs\" has the form \"-j\", and it takes a numerical argument.\n>\n> When parsing short options, support the case where there is no space\n> between the flag and the option argument, in order to improve\n> consistency with the rest of the builtin git commands.\n>\n> Signed-off-by: Roy Eldar \n> ---\n> git-submodule.sh | 13 +++++++++++++\n> 1 file changed, 13 insertions(+)\n>\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index a47d2a89f3..fc85458fb1 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -77,6 +77,9 @@ cmd_add()\n> \t\t\tbranch=$2\n> \t\t\tshift\n> \t\t\t;;\n> +\t\t-b*)\n> +\t\t\tbranch=\"${1#-b}\"\n> +\t\t\t;;\n> \t\t--branch=*)\n> \t\t\tbranch=\"${1#--branch=}\"\n> \t\t\t;;\n\nOK, so we used to take \"--branch foo\" and assign 'foo' to $branch,\n\"--branch=foo\" now does the same, and then we have \"-b foo\" doing\nthe same with this step.\n\n> @@ -352,6 +355,9 @@ cmd_update()\n> \t\t\tjobs=\"--jobs=$2\"\n> \t\t\tshift\n> \t\t\t;;\n> +\t\t-j*)\n> +\t\t\tjobs=\"--jobs=${1#-j}\"\n> +\t\t\t;;\n> \t\t--jobs=*)\n> \t\t\tjobs=$1\n> \t\t\t;;\n\nThis is a bit curious. If you stick the principle in 1/3, this\nshould assign \"4\", not \"--jobs=4\", to variable $jobs upon seeing\n\"-j4\", and that would be consistent with how the $branch gets its\nvalue above.\n\nAs I said in the devil's advocate section in my review for 1/3, I\noften find the value of the variable spelling out the option name\nas well as the option value (i.e., force=\"--force\", not force=1;\nbranch=\"--branch=foo\", not branch=foo; jobs=--jobs=4, not jobs=4)\neasier to debug and drive other programs using these variables, so I\ndo not mind jobs=--jobs=4 at all, but if we want to be consistent in\nthe other direction, this would probably want to be modified in the\nname of consistency?\n\nOther than that, all three patches looked sane to me.\n\nThanks.\n\n\n\t\n"},{"id":"508827","messageId":"CAPig+cS9tJ1YjGyCObmgrruv816szhzA57PJNBLghEiYj1Qs7A@mail.gmail.com","threadId":"62611","inReplyTo":"20241207135201.2536-2-royeldar0@gmail.com","subject":"Re: [PATCH 1/3] git-submodule.sh: make some variables boolean","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-12-08T00:06:07Z","receivedAt":"2024-12-08T00:06:20Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Dec 7, 2024 at 8:53 AM Roy Eldar wrote:\n> When git-submodule.sh parses various options and switchs, it sets some\n\ns/switchs/switches/\n\n> variables to values; in particular, every switch that is passed causes a\n> corresponding variable to be set to 1, which then affects the options\n> given to git-submodule--helper.\n>\n> There are some variables are assigned \"$1\", although there is no reason\n\ns/are assigned/assigned/\n\n> for it; this was actually noticed in 757d092 for the \"$cached\" variable.\n>\n> Make some variables boolean, in order to increase consistency throught\n\ns/throught/throughout/\n\n> the script and reduce possible confusion.\n>\n> Signed-off-by: Roy Eldar \n"},{"id":"508870","messageId":"CAOfFammyK1Ef4_gOV+a5hiZR+Xhgu0o+=1Uu4BE2bGUACo3Jtg@mail.gmail.com","threadId":"62611","inReplyTo":"xmqqr06jrqiv.fsf@gitster.g","subject":"Re: [PATCH 3/3] git-submodule.sh: improve parsing of short options","fromName":"Roy E","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-09T16:21:06Z","receivedAt":"2024-12-09T16:21:18Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"Hi,\n\nJunio C Hamano wrote:\n\n> As I said in the devil's advocate section in my review for 1/3, I\n> often find the value of the variable spelling out the option name\n> as well as the option value (i.e., force=\"--force\", not force=1;\n> branch=\"--branch=foo\", not branch=foo; jobs=--jobs=4, not jobs=4)\n> easier to debug and drive other programs using these variables, so I\n> do not mind jobs=--jobs=4 at all, but if we want to be consistent in\n> the other direction, this would probably want to be modified in the\n> name of consistency?\n\nI find this approach better as well, and I agree this is easier to read,\nand has some other advantages like better ability to debug the script.\nI modified the script so that all of the variables are assigned values\nwhich contain the option name as well as the option value, and I will\npost a v2 patch series soon.\n\n> Other than that, all three patches looked sane to me.\n\nThanks a lot!\n"},{"id":"508872","messageId":"20241209165009.40653-1-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241207135201.2536-1-royeldar0@gmail.com","subject":"[PATCH v2 0/8] git-submodule.sh: improve parsing of options","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-09T16:50:01Z","receivedAt":"2024-12-09T16:51:17Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"When we run \"git submodule\", the script parses the various options and\nthen invokes \"git-submodule--helper\". Unlike most builtin git commands\nwhich parse short/long options using parse-options.c, the parsing of\narguments is completely done within git-submodule.sh; therefore, there\nare some inconsistencies with the rest of the commands, in particular\nthe parsing of option arguments given to various options.\n\nImprove the handling of option arguments for both long & short options;\nfor example, passing flags such as \"--branch=master\" or \"-j8\" now works.\n\nChanges since v1:\n\n- Make variable values always contain the option name.\n- Rename a couple of variables in order to improve consistency.\n\nLink to v1:\n\nhttps://lore.kernel.org/git/20241207135201.2536-1-royeldar0@gmail.com\n\nRoy Eldar (8):\n git-submodule.sh: make some variables boolean\n git-submodule.sh: improve parsing of some long options\n git-submodule.sh: improve parsing of short options\n git-submodule.sh: get rid of isnumber\n git-submodule.sh: get rid of unused variable\n git-submodule.sh: add some comments\n git-submodule.sh: improve variables readability\n git-submodule.sh: rename some variables\n\n git-submodule.sh | 214 +++++++++++++++++++++++------------------------\n 1 file changed, 104 insertions(+), 110 deletions(-)\n\n-- \n2.30.2\n\n"},{"id":"508873","messageId":"20241209165009.40653-2-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241209165009.40653-1-royeldar0@gmail.com","subject":"[PATCH v2 1/8] git-submodule.sh: make some variables boolean","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-09T16:50:02Z","receivedAt":"2024-12-09T16:51:20Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"When git-submodule.sh parses various options and switches, it sets some\nvariables to values; in particular, every switch that is passed causes a\ncorresponding variable to be set to 1, which then affects the options\ngiven to git-submodule--helper.\n\nSome variables are assigned \"$1\", although there is no reason for it;\nthis was actually noticed in 757d092 for the \"$cached\" variable.\n\nMake some variables boolean, in order to increase consistency throughout\nthe script and reduce possible confusion.\n\nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 03c5a220a2..107011f613 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -78,7 +78,7 @@ cmd_add()\n \t\t\tshift\n \t\t\t;;\n \t\t-f | --force)\n-\t\t\tforce=$1\n+\t\t\tforce=1\n \t\t\t;;\n \t\t-q|--quiet)\n \t\t\tquiet=1\n@@ -231,7 +231,7 @@ cmd_deinit()\n \tdo\n \t\tcase \"$1\" in\n \t\t-f|--force)\n-\t\t\tforce=$1\n+\t\t\tforce=1\n \t\t\t;;\n \t\t-q|--quiet)\n \t\t\tquiet=1\n@@ -294,7 +294,7 @@ cmd_update()\n \t\t\tnofetch=1\n \t\t\t;;\n \t\t-f|--force)\n-\t\t\tforce=$1\n+\t\t\tforce=1\n \t\t\t;;\n \t\t-r|--rebase)\n \t\t\trebase=1\n@@ -500,10 +500,10 @@ cmd_summary() {\n \t\t\tcached=1\n \t\t\t;;\n \t\t--files)\n-\t\t\tfiles=\"$1\"\n+\t\t\tfiles=1\n \t\t\t;;\n \t\t--for-status)\n-\t\t\tfor_status=\"$1\"\n+\t\t\tfor_status=1\n \t\t\t;;\n \t\t-n|--summary-limit)\n \t\t\tsummary_limit=\"$2\"\n-- \n2.30.2\n\n"},{"id":"508874","messageId":"20241209165009.40653-3-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241209165009.40653-1-royeldar0@gmail.com","subject":"[PATCH v2 2/8] git-submodule.sh: improve parsing of some long options","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-09T16:50:03Z","receivedAt":"2024-12-09T16:51:23Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"Some command-line options have a long form which takes an argument. In\nthis case, the argument can be given right after `='; for example,\n\"--depth\" takes a numerical argument, which can be given as \"--depth=X\".\n\nSupport the case where the argument is given right after `=' for all\nlong options, in order to improve consistency throughout the script.\n\nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 9 +++++++++\n 1 file changed, 9 insertions(+)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 107011f613..a47d2a89f3 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -77,6 +77,9 @@ cmd_add()\n \t\t\tbranch=$2\n \t\t\tshift\n \t\t\t;;\n+\t\t--branch=*)\n+\t\t\tbranch=\"${1#--branch=}\"\n+\t\t\t;;\n \t\t-f | --force)\n \t\t\tforce=1\n \t\t\t;;\n@@ -110,6 +113,9 @@ cmd_add()\n \t\t\tcustom_name=$2\n \t\t\tshift\n \t\t\t;;\n+\t\t--name=*)\n+\t\t\tcustom_name=\"${1#--name=}\"\n+\t\t\t;;\n \t\t--depth)\n \t\t\tcase \"$2\" in '') usage ;; esac\n \t\t\tdepth=\"--depth=$2\"\n@@ -425,6 +431,9 @@ cmd_set_branch() {\n \t\t\tbranch=$2\n \t\t\tshift\n \t\t\t;;\n+\t\t--branch=*)\n+\t\t\tbranch=\"${1#--branch=}\"\n+\t\t\t;;\n \t\t--)\n \t\t\tshift\n \t\t\tbreak\n-- \n2.30.2\n\n"},{"id":"508875","messageId":"20241209165009.40653-4-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241209165009.40653-1-royeldar0@gmail.com","subject":"[PATCH v2 3/8] git-submodule.sh: improve parsing of short options","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-09T16:50:04Z","receivedAt":"2024-12-09T16:51:25Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"Some command-line options have a short form which takes an argument; for\nexample, \"--jobs\" has the form \"-j\", and it takes a numerical argument.\n\nWhen parsing short options, support the case where there is no space\nbetween the flag and the option argument, in order to improve\nconsistency with the rest of the builtin git commands.\n\nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 13 +++++++++++++\n 1 file changed, 13 insertions(+)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex a47d2a89f3..fc85458fb1 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -77,6 +77,9 @@ cmd_add()\n \t\t\tbranch=$2\n \t\t\tshift\n \t\t\t;;\n+\t\t-b*)\n+\t\t\tbranch=\"${1#-b}\"\n+\t\t\t;;\n \t\t--branch=*)\n \t\t\tbranch=\"${1#--branch=}\"\n \t\t\t;;\n@@ -352,6 +355,9 @@ cmd_update()\n \t\t\tjobs=\"--jobs=$2\"\n \t\t\tshift\n \t\t\t;;\n+\t\t-j*)\n+\t\t\tjobs=\"--jobs=${1#-j}\"\n+\t\t\t;;\n \t\t--jobs=*)\n \t\t\tjobs=$1\n \t\t\t;;\n@@ -431,6 +437,9 @@ cmd_set_branch() {\n \t\t\tbranch=$2\n \t\t\tshift\n \t\t\t;;\n+\t\t-b*)\n+\t\t\tbranch=\"${1#-b}\"\n+\t\t\t;;\n \t\t--branch=*)\n \t\t\tbranch=\"${1#--branch=}\"\n \t\t\t;;\n@@ -519,6 +528,10 @@ cmd_summary() {\n \t\t\tisnumber \"$summary_limit\" || usage\n \t\t\tshift\n \t\t\t;;\n+\t\t-n*)\n+\t\t\tsummary_limit=\"${1#-n}\"\n+\t\t\tisnumber \"$summary_limit\" || usage\n+\t\t\t;;\n \t\t--summary-limit=*)\n \t\t\tsummary_limit=\"${1#--summary-limit=}\"\n \t\t\tisnumber \"$summary_limit\" || usage\n-- \n2.30.2\n\n"},{"id":"508876","messageId":"20241209165009.40653-5-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241209165009.40653-1-royeldar0@gmail.com","subject":"[PATCH v2 4/8] git-submodule.sh: get rid of isnumber","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-09T16:50:05Z","receivedAt":"2024-12-09T16:51:27Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"It's entirely unnecessary to check whether the argument given to an\noption (i.e. --summary-limit) is valid in the shell wrapper, since it's\nalready done when parsing the various options in git-submodule--helper.\n\nRemove this check from the script; this both improves consistency\nthroughout the script, and the error message shown to the user in case\nsome invalid non-numeric argument was passed to \"--summary-limit\" is\nmore informative as well.\n\nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 11 ++---------\n 1 file changed, 2 insertions(+), 9 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex fc85458fb1..833ac8362b 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -53,11 +53,6 @@ jobs=\n recommend_shallow=\n filter=\n \n-isnumber()\n-{\n-\tn=$(($1 + 0)) 2>/dev/null && test \"$n\" = \"$1\"\n-}\n-\n #\n # Add a new submodule to the working tree, .gitmodules and the index\n #\n@@ -524,17 +519,15 @@ cmd_summary() {\n \t\t\tfor_status=1\n \t\t\t;;\n \t\t-n|--summary-limit)\n+\t\t\tcase \"$2\" in '') usage ;; esac\n \t\t\tsummary_limit=\"$2\"\n-\t\t\tisnumber \"$summary_limit\" || usage\n \t\t\tshift\n \t\t\t;;\n \t\t-n*)\n \t\t\tsummary_limit=\"${1#-n}\"\n-\t\t\tisnumber \"$summary_limit\" || usage\n \t\t\t;;\n \t\t--summary-limit=*)\n \t\t\tsummary_limit=\"${1#--summary-limit=}\"\n-\t\t\tisnumber \"$summary_limit\" || usage\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -554,7 +547,7 @@ cmd_summary() {\n \t\t${files:+--files} \\\n \t\t${cached:+--cached} \\\n \t\t${for_status:+--for-status} \\\n-\t\t${summary_limit:+-n $summary_limit} \\\n+\t\t${summary_limit:+-n \"$summary_limit\"} \\\n \t\t-- \\\n \t\t\"$@\"\n }\n-- \n2.30.2\n\n"},{"id":"508877","messageId":"20241209165009.40653-6-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241209165009.40653-1-royeldar0@gmail.com","subject":"[PATCH v2 5/8] git-submodule.sh: get rid of unused variable","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-09T16:50:06Z","receivedAt":"2024-12-09T16:51:29Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"Remove the variable \"$diff_cmd\" which is no longer used.\n\nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 833ac8362b..67cdea331b 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -503,7 +503,6 @@ cmd_set_url() {\n cmd_summary() {\n \tsummary_limit=-1\n \tfor_status=\n-\tdiff_cmd=diff-index\n \n \t# parse $args after \"submodule ... summary\".\n \twhile test $# -ne 0\n-- \n2.30.2\n\n"},{"id":"508878","messageId":"20241209165009.40653-7-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241209165009.40653-1-royeldar0@gmail.com","subject":"[PATCH v2 6/8] git-submodule.sh: add some comments","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-09T16:50:07Z","receivedAt":"2024-12-09T16:51:31Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"Add a couple of comments in a few functions where they were missing.\n\nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 67cdea331b..3b44aeddbf 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -418,6 +418,7 @@ cmd_set_branch() {\n \tdefault=\n \tbranch=\n \n+\t# parse $args after \"submodule ... set-branch\".\n \twhile test $# -ne 0\n \tdo\n \t\tcase \"$1\" in\n@@ -466,6 +467,7 @@ cmd_set_branch() {\n # $@ = requested path, requested url\n #\n cmd_set_url() {\n+\t# parse $args after \"submodule ... set-url\".\n \twhile test $# -ne 0\n \tdo\n \t\tcase \"$1\" in\n@@ -604,6 +606,7 @@ cmd_status()\n #\n cmd_sync()\n {\n+\t# parse $args after \"submodule ... sync\".\n \twhile test $# -ne 0\n \tdo\n \t\tcase \"$1\" in\n-- \n2.30.2\n\n"},{"id":"508879","messageId":"20241209165009.40653-8-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241209165009.40653-1-royeldar0@gmail.com","subject":"[PATCH v2 7/8] git-submodule.sh: improve variables readability","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-09T16:50:08Z","receivedAt":"2024-12-09T16:51:32Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"When git-submodule.sh parses various options and switches, it sets some\nvariables to values; the variables in turn affect the options given to\ngit-submodule--helper.\n\nCurrently, variables which correspond to switches have boolean values\n(for example, whenever \"--force\" is passed, force=1), while variables\nwhich correspond to options which take arguments have string values that\nsometimes contain the option name and sometimes only the option value.\n\nSet all of the variables to strings which contain the option name (e.g.\nforce=\"--force\" rather than force=1); this has a couple of advantages:\nit improves consistency, readability and debuggability.\n\nSuggested-by: Junio C Hamano \nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 219 +++++++++++++++++++++--------------------------\n 1 file changed, 98 insertions(+), 121 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 3b44aeddbf..2da4d55d64 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -52,6 +52,10 @@ single_branch=\n jobs=\n recommend_shallow=\n filter=\n+deinit_all=\n+default=\n+summary_limit=\n+for_status=\n \n #\n # Add a new submodule to the working tree, .gitmodules and the index\n@@ -63,37 +67,33 @@ filter=\n cmd_add()\n {\n \t# parse $args after \"submodule ... add\".\n-\treference_path=\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\tbranch=\"--branch=$2\"\n \t\t\tshift\n \t\t\t;;\n-\t\t-b*)\n-\t\t\tbranch=\"${1#-b}\"\n-\t\t\t;;\n-\t\t--branch=*)\n-\t\t\tbranch=\"${1#--branch=}\"\n+\t\t-b* | --branch*)\n+\t\t\tbranch=\"$1\"\n \t\t\t;;\n \t\t-f | --force)\n-\t\t\tforce=1\n+\t\t\tforce=$1\n \t\t\t;;\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\t;;\n \t\t--progress)\n-\t\t\tprogress=1\n+\t\t\tprogress=$1\n \t\t\t;;\n \t\t--reference)\n \t\t\tcase \"$2\" in '') usage ;; esac\n-\t\t\treference_path=$2\n+\t\t\treference=\"--reference=$2\"\n \t\t\tshift\n \t\t\t;;\n \t\t--reference=*)\n-\t\t\treference_path=\"${1#--reference=}\"\n+\t\t\treference=\"$1\"\n \t\t\t;;\n \t\t--ref-format)\n \t\t\tcase \"$2\" in '') usage ;; esac\n@@ -104,15 +104,15 @@ cmd_add()\n \t\t\tref_format=\"$1\"\n \t\t\t;;\n \t\t--dissociate)\n-\t\t\tdissociate=1\n+\t\t\tdissociate=$1\n \t\t\t;;\n \t\t--name)\n \t\t\tcase \"$2\" in '') usage ;; esac\n-\t\t\tcustom_name=$2\n+\t\t\tcustom_name=\"--name=$2\"\n \t\t\tshift\n \t\t\t;;\n \t\t--name=*)\n-\t\t\tcustom_name=\"${1#--name=}\"\n+\t\t\tcustom_name=\"$1\"\n \t\t\t;;\n \t\t--depth)\n \t\t\tcase \"$2\" in '') usage ;; esac\n@@ -120,7 +120,7 @@ cmd_add()\n \t\t\tshift\n \t\t\t;;\n \t\t--depth=*)\n-\t\t\tdepth=$1\n+\t\t\tdepth=\"$1\"\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -142,14 +142,14 @@ cmd_add()\n \tfi\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper add \\\n-\t\t${quiet:+--quiet} \\\n-\t\t${force:+--force} \\\n-\t\t${progress:+\"--progress\"} \\\n-\t\t${branch:+--branch \"$branch\"} \\\n-\t\t${reference_path:+--reference \"$reference_path\"} \\\n+\t\t$quiet \\\n+\t\t$force \\\n+\t\t$progress \\\n+\t\t${branch:+\"$branch\"} \\\n+\t\t${reference:+\"$reference\"} \\\n \t\t${ref_format:+\"$ref_format\"} \\\n-\t\t${dissociate:+--dissociate} \\\n-\t\t${custom_name:+--name \"$custom_name\"} \\\n+\t\t$dissociate \\\n+\t\t${custom_name:+\"$custom_name\"} \\\n \t\t${depth:+\"$depth\"} \\\n \t\t-- \\\n \t\t\"$@\"\n@@ -168,10 +168,10 @@ cmd_foreach()\n \tdo\n \t\tcase \"$1\" in\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\t;;\n \t\t--recursive)\n-\t\t\trecursive=1\n+\t\t\trecursive=$1\n \t\t\t;;\n \t\t-*)\n \t\t\tusage\n@@ -184,8 +184,8 @@ cmd_foreach()\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper foreach \\\n-\t\t${quiet:+--quiet} \\\n-\t\t${recursive:+--recursive} \\\n+\t\t$quiet \\\n+\t\t$recursive \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -202,7 +202,7 @@ cmd_init()\n \tdo\n \t\tcase \"$1\" in\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -219,7 +219,7 @@ cmd_init()\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper init \\\n-\t\t${quiet:+--quiet} \\\n+\t\t$quiet \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -230,18 +230,17 @@ cmd_init()\n cmd_deinit()\n {\n \t# parse $args after \"submodule ... deinit\".\n-\tdeinit_all=\n \twhile test $# -ne 0\n \tdo\n \t\tcase \"$1\" in\n \t\t-f|--force)\n-\t\t\tforce=1\n+\t\t\tforce=$1\n \t\t\t;;\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\t;;\n \t\t--all)\n-\t\t\tdeinit_all=t\n+\t\t\tdeinit_all=$1\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -258,9 +257,9 @@ cmd_deinit()\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper deinit \\\n-\t\t${quiet:+--quiet} \\\n-\t\t${force:+--force} \\\n-\t\t${deinit_all:+--all} \\\n+\t\t$quiet \\\n+\t\t$force \\\n+\t\t$deinit_all \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -277,31 +276,31 @@ cmd_update()\n \tdo\n \t\tcase \"$1\" in\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\t;;\n \t\t-v|--verbose)\n-\t\t\tquiet=0\n+\t\t\tquiet=\n \t\t\t;;\n \t\t--progress)\n-\t\t\tprogress=1\n+\t\t\tprogress=$1\n \t\t\t;;\n \t\t-i|--init)\n-\t\t\tinit=1\n+\t\t\tinit=$1\n \t\t\t;;\n \t\t--require-init)\n-\t\t\trequire_init=1\n+\t\t\trequire_init=$1\n \t\t\t;;\n \t\t--remote)\n-\t\t\tremote=1\n+\t\t\tremote=$1\n \t\t\t;;\n \t\t-N|--no-fetch)\n-\t\t\tnofetch=1\n+\t\t\tnofetch=$1\n \t\t\t;;\n \t\t-f|--force)\n-\t\t\tforce=1\n+\t\t\tforce=$1\n \t\t\t;;\n \t\t-r|--rebase)\n-\t\t\trebase=1\n+\t\t\trebase=$1\n \t\t\t;;\n \t\t--ref-format)\n \t\t\tcase \"$2\" in '') usage ;; esac\n@@ -320,22 +319,19 @@ cmd_update()\n \t\t\treference=\"$1\"\n \t\t\t;;\n \t\t--dissociate)\n-\t\t\tdissociate=1\n+\t\t\tdissociate=$1\n \t\t\t;;\n \t\t-m|--merge)\n-\t\t\tmerge=1\n+\t\t\tmerge=$1\n \t\t\t;;\n \t\t--recursive)\n-\t\t\trecursive=1\n+\t\t\trecursive=$1\n \t\t\t;;\n \t\t--checkout)\n-\t\t\tcheckout=1\n-\t\t\t;;\n-\t\t--recommend-shallow)\n-\t\t\trecommend_shallow=\"--recommend-shallow\"\n+\t\t\tcheckout=$1\n \t\t\t;;\n-\t\t--no-recommend-shallow)\n-\t\t\trecommend_shallow=\"--no-recommend-shallow\"\n+\t\t--recommend-shallow|--no-recommend-shallow)\n+\t\t\trecommend_shallow=$1\n \t\t\t;;\n \t\t--depth)\n \t\t\tcase \"$2\" in '') usage ;; esac\n@@ -343,24 +339,18 @@ cmd_update()\n \t\t\tshift\n \t\t\t;;\n \t\t--depth=*)\n-\t\t\tdepth=$1\n+\t\t\tdepth=\"$1\"\n \t\t\t;;\n \t\t-j|--jobs)\n \t\t\tcase \"$2\" in '') usage ;; esac\n \t\t\tjobs=\"--jobs=$2\"\n \t\t\tshift\n \t\t\t;;\n-\t\t-j*)\n-\t\t\tjobs=\"--jobs=${1#-j}\"\n-\t\t\t;;\n-\t\t--jobs=*)\n-\t\t\tjobs=$1\n+\t\t-j*|--jobs*)\n+\t\t\tjobs=\"$1\"\n \t\t\t;;\n-\t\t--single-branch)\n-\t\t\tsingle_branch=\"--single-branch\"\n-\t\t\t;;\n-\t\t--no-single-branch)\n-\t\t\tsingle_branch=\"--no-single-branch\"\n+\t\t--single-branch|--no-single-branch)\n+\t\t\tsingle_branch=$1\n \t\t\t;;\n \t\t--filter)\n \t\t\tcase \"$2\" in '') usage ;; esac\n@@ -385,22 +375,21 @@ cmd_update()\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper update \\\n-\t\t${quiet:+--quiet} \\\n-\t\t${force:+--force} \\\n-\t\t${progress:+\"--progress\"} \\\n-\t\t${remote:+--remote} \\\n-\t\t${recursive:+--recursive} \\\n-\t\t${init:+--init} \\\n-\t\t${nofetch:+--no-fetch} \\\n-\t\t${rebase:+--rebase} \\\n-\t\t${merge:+--merge} \\\n-\t\t${checkout:+--checkout} \\\n+\t\t$quiet \\\n+\t\t$force \\\n+\t\t$progress \\\n+\t\t$remote \\\n+\t\t$recursive \\\n+\t\t$init \\\n+\t\t$nofetch \\\n+\t\t$rebase \\\n+\t\t$merge \\\n+\t\t$checkout \\\n \t\t${ref_format:+\"$ref_format\"} \\\n \t\t${reference:+\"$reference\"} \\\n-\t\t${dissociate:+\"--dissociate\"} \\\n+\t\t$dissociate \\\n \t\t${depth:+\"$depth\"} \\\n-\t\t${require_init:+--require-init} \\\n-\t\t${dissociate:+\"--dissociate\"} \\\n+\t\t$require_init \\\n \t\t$single_branch \\\n \t\t$recommend_shallow \\\n \t\t$jobs \\\n@@ -415,9 +404,6 @@ cmd_update()\n # $@ = requested path\n #\n cmd_set_branch() {\n-\tdefault=\n-\tbranch=\n-\n \t# parse $args after \"submodule ... set-branch\".\n \twhile test $# -ne 0\n \tdo\n@@ -426,18 +412,15 @@ cmd_set_branch() {\n \t\t\t# we don't do anything with this but we need to accept it\n \t\t\t;;\n \t\t-d|--default)\n-\t\t\tdefault=1\n+\t\t\tdefault=$1\n \t\t\t;;\n \t\t-b|--branch)\n \t\t\tcase \"$2\" in '') usage ;; esac\n-\t\t\tbranch=$2\n+\t\t\tbranch=\"--branch=$2\"\n \t\t\tshift\n \t\t\t;;\n-\t\t-b*)\n-\t\t\tbranch=\"${1#-b}\"\n-\t\t\t;;\n-\t\t--branch=*)\n-\t\t\tbranch=\"${1#--branch=}\"\n+\t\t-b*|--branch=*)\n+\t\t\tbranch=\"$1\"\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -454,9 +437,9 @@ cmd_set_branch() {\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper set-branch \\\n-\t\t${quiet:+--quiet} \\\n-\t\t${branch:+--branch \"$branch\"} \\\n-\t\t${default:+--default} \\\n+\t\t$quiet \\\n+\t\t${branch:+\"$branch\"} \\\n+\t\t$default \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -472,7 +455,7 @@ cmd_set_url() {\n \tdo\n \t\tcase \"$1\" in\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -489,7 +472,7 @@ cmd_set_url() {\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper set-url \\\n-\t\t${quiet:+--quiet} \\\n+\t\t$quiet \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -503,32 +486,26 @@ cmd_set_url() {\n # $@ = [commit (default 'HEAD'),] requested paths (default all)\n #\n cmd_summary() {\n-\tsummary_limit=-1\n-\tfor_status=\n-\n \t# parse $args after \"submodule ... summary\".\n \twhile test $# -ne 0\n \tdo\n \t\tcase \"$1\" in\n \t\t--cached)\n-\t\t\tcached=1\n+\t\t\tcached=$1\n \t\t\t;;\n \t\t--files)\n-\t\t\tfiles=1\n+\t\t\tfiles=$1\n \t\t\t;;\n \t\t--for-status)\n-\t\t\tfor_status=1\n+\t\t\tfor_status=$1\n \t\t\t;;\n \t\t-n|--summary-limit)\n \t\t\tcase \"$2\" in '') usage ;; esac\n-\t\t\tsummary_limit=\"$2\"\n+\t\t\tsummary_limit=\"--summary-limit=$2\"\n \t\t\tshift\n \t\t\t;;\n-\t\t-n*)\n-\t\t\tsummary_limit=\"${1#-n}\"\n-\t\t\t;;\n-\t\t--summary-limit=*)\n-\t\t\tsummary_limit=\"${1#--summary-limit=}\"\n+\t\t-n*|--summary-limit=*)\n+\t\t\tsummary_limit=\"$1\"\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -545,10 +522,10 @@ cmd_summary() {\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper summary \\\n-\t\t${files:+--files} \\\n-\t\t${cached:+--cached} \\\n-\t\t${for_status:+--for-status} \\\n-\t\t${summary_limit:+-n \"$summary_limit\"} \\\n+\t\t$files \\\n+\t\t$cached \\\n+\t\t$for_status \\\n+\t\t${summary_limit:+\"$summary_limit\"} \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -569,13 +546,13 @@ cmd_status()\n \tdo\n \t\tcase \"$1\" in\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\t;;\n \t\t--cached)\n-\t\t\tcached=1\n+\t\t\tcached=$1\n \t\t\t;;\n \t\t--recursive)\n-\t\t\trecursive=1\n+\t\t\trecursive=$1\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -592,9 +569,9 @@ cmd_status()\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper status \\\n-\t\t${quiet:+--quiet} \\\n-\t\t${cached:+--cached} \\\n-\t\t${recursive:+--recursive} \\\n+\t\t$quiet \\\n+\t\t$cached \\\n+\t\t$recursive \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -611,11 +588,11 @@ cmd_sync()\n \tdo\n \t\tcase \"$1\" in\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\tshift\n \t\t\t;;\n \t\t--recursive)\n-\t\t\trecursive=1\n+\t\t\trecursive=$1\n \t\t\tshift\n \t\t\t;;\n \t\t--)\n@@ -632,8 +609,8 @@ cmd_sync()\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper sync \\\n-\t\t${quiet:+--quiet} \\\n-\t\t${recursive:+--recursive} \\\n+\t\t$quiet \\\n+\t\t$recursive \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -656,10 +633,10 @@ do\n \t\tcommand=$1\n \t\t;;\n \t-q|--quiet)\n-\t\tquiet=1\n+\t\tquiet=$1\n \t\t;;\n \t--cached)\n-\t\tcached=1\n+\t\tcached=$1\n \t\t;;\n \t--)\n \t\tbreak\n-- \n2.30.2\n\n"},{"id":"508880","messageId":"20241209165009.40653-9-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241209165009.40653-1-royeldar0@gmail.com","subject":"[PATCH v2 8/8] git-submodule.sh: rename some variables","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-09T16:50:09Z","receivedAt":"2024-12-09T16:51:34Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"Every switch and option which is passed to git-submodule.sh has a\ncorresponding variable which is set accordingly; by convention, the name\nof the variable is the option name (for example, \"--jobs\" and \"$jobs\").\n\nRename \"$custom_name\" and \"$deinit_all\" in order to improve consistency.\n\nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 2da4d55d64..9c10472b5f 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -44,7 +44,7 @@ nofetch=\n rebase=\n merge=\n checkout=\n-custom_name=\n+name=\n depth=\n progress=\n dissociate=\n@@ -52,7 +52,7 @@ single_branch=\n jobs=\n recommend_shallow=\n filter=\n-deinit_all=\n+all=\n default=\n summary_limit=\n for_status=\n@@ -108,11 +108,11 @@ cmd_add()\n \t\t\t;;\n \t\t--name)\n \t\t\tcase \"$2\" in '') usage ;; esac\n-\t\t\tcustom_name=\"--name=$2\"\n+\t\t\tname=\"--name=$2\"\n \t\t\tshift\n \t\t\t;;\n \t\t--name=*)\n-\t\t\tcustom_name=\"$1\"\n+\t\t\tname=\"$1\"\n \t\t\t;;\n \t\t--depth)\n \t\t\tcase \"$2\" in '') usage ;; esac\n@@ -149,7 +149,7 @@ cmd_add()\n \t\t${reference:+\"$reference\"} \\\n \t\t${ref_format:+\"$ref_format\"} \\\n \t\t$dissociate \\\n-\t\t${custom_name:+\"$custom_name\"} \\\n+\t\t${name:+\"$name\"} \\\n \t\t${depth:+\"$depth\"} \\\n \t\t-- \\\n \t\t\"$@\"\n@@ -240,7 +240,7 @@ cmd_deinit()\n \t\t\tquiet=$1\n \t\t\t;;\n \t\t--all)\n-\t\t\tdeinit_all=$1\n+\t\t\tall=$1\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -259,7 +259,7 @@ cmd_deinit()\n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper deinit \\\n \t\t$quiet \\\n \t\t$force \\\n-\t\t$deinit_all \\\n+\t\t$all \\\n \t\t-- \\\n \t\t\"$@\"\n }\n-- \n2.30.2\n\n"},{"id":"508892","messageId":"xmqq1pygo2ti.fsf@gitster.g","threadId":"62611","inReplyTo":"20241209165009.40653-1-royeldar0@gmail.com","subject":"Re: [PATCH v2 0/8] git-submodule.sh: improve parsing of options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-09T23:26:49Z","receivedAt":"2024-12-09T23:26:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Roy Eldar writes:\n\n> When we run \"git submodule\", the script parses the various options and\n> then invokes \"git-submodule--helper\". Unlike most builtin git commands\n> which parse short/long options using parse-options.c, the parsing of\n> arguments is completely done within git-submodule.sh; therefore, there\n> are some inconsistencies with the rest of the commands, in particular\n> the parsing of option arguments given to various options.\n>\n> Improve the handling of option arguments for both long & short options;\n> for example, passing flags such as \"--branch=master\" or \"-j8\" now works.\n>\n> Changes since v1:\n>\n> - Make variable values always contain the option name.\n> - Rename a couple of variables in order to improve consistency.\n\nAfter reading this, it was confusing to see [1/8] still doing \"1 or\nempty\" boolean, only to be further modified in [7/8]. We prefer to\nsee a single series stumbling in the middle and changing the course.\n\nJust a tangent.\n\nWhile a simple wrapper script is generally easier to debug and read\nif $verbose variable's value is \"--verbose\" or \"\", there is a case\nwhere following this pattern is not a good idea. If an option we\nare eventually going to give to the underlying command takes a\nvalue, the value can contain whitespace, and the option and its\nvalue need to be passed as two separate arguments, it is less error\nprone to use the \"variable only contains the value\" approach.\n\nImagine that submodule--helper takes a \"--foo\" option with a greeting\nmessage like \"hello world\" in such a way. We'd want to trigger it\nthis way:\n\n\tgit submodule--helper --foo \"hello world\"\n\nas we are assuming that for some reason we need to pass them as two\nwords, and\n\n\tgit submodule--helper --foo=\"hello world\"\n\nis not an option. In such a case, a wrapper script that takes such\nan optional parameter in $foo is easier to write like so\n\n\t# parse\n\tfoo=\n\twhile ...\n\tdo\n case \"$opt\" in\n --foo=*)\tfoo=\"${1#--foo=}\" ;;\n --foo)\t\tfoo=${2?\"--foo without value\"};\tshift ;;\n\t\t...\n\t\tesac\n\t\tshift\n\tdone\n\n\t# interpolate\n\tgit submodule--helper ${foo:+--foo \"$foo\"}\n\nin order to avoid the value given to the option split at $IFS\nwhitespace. With foo='--foo=\"hello world\"', passing it to the\nunderlying command would involve use of eval and becomes error\nprone.\n\nI am assuming (but I don't use \"git submodule\" very often, so my\nassumption may be way off) that there is no such variable we need to\npass, but if not, we may need to reconsider and use the \"variable has\nonly value of the option\" for at least some of them.\n\n\n\n\n\n> Link to v1:\n>\n> https://lore.kernel.org/git/20241207135201.2536-1-royeldar0@gmail.com\n>\n> Roy Eldar (8):\n> git-submodule.sh: make some variables boolean\n> git-submodule.sh: improve parsing of some long options\n> git-submodule.sh: improve parsing of short options\n> git-submodule.sh: get rid of isnumber\n> git-submodule.sh: get rid of unused variable\n> git-submodule.sh: add some comments\n> git-submodule.sh: improve variables readability\n> git-submodule.sh: rename some variables\n>\n> git-submodule.sh | 214 +++++++++++++++++++++++------------------------\n> 1 file changed, 104 insertions(+), 110 deletions(-)\n"},{"id":"508898","messageId":"xmqqa5d4mkdf.fsf@gitster.g","threadId":"62611","inReplyTo":"xmqq1pygo2ti.fsf@gitster.g","subject":"Re: [PATCH v2 0/8] git-submodule.sh: improve parsing of options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-10T00:50:36Z","receivedAt":"2024-12-10T00:50:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> After reading this, it was confusing to see [1/8] still doing \"1 or\n> empty\" boolean, only to be further modified in [7/8]. We prefer to\n> see a single series stumbling in the middle and changing the course.\n\nFacepalm. Of course, we prefer _not_ to see such a change of course\nin the middle.\n\nThanks.\n\n> I am assuming (but I don't use \"git submodule\" very often, so my\n> assumption may be way off) that there is no such variable we need to\n> pass, but if not, we may need to reconsider and use the \"variable has\n> only value of the option\" for at least some of them.\n\n"},{"id":"508930","messageId":"CAOfFamkEmJgUUj8C_TGbjgKs35vSBzNCDpLvK+utqnTtxd5iHQ@mail.gmail.com","threadId":"62611","inReplyTo":"xmqq1pygo2ti.fsf@gitster.g","subject":"Re: [PATCH v2 0/8] git-submodule.sh: improve parsing of options","fromName":"Roy E","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-10T18:11:53Z","receivedAt":"2024-12-10T18:12:05Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"Junio C Hamano wrote:\n\n> After reading this, it was confusing to see [1/8] still doing \"1 or\n> empty\" boolean, only to be further modified in [7/8]. We prefer to\n> see a single series stumbling in the middle and changing the course.\n\nOK, I will remove this patch from the series and post a v3 series soon.\n\n> While a simple wrapper script is generally easier to debug and read\n> if $verbose variable's value is \"--verbose\" or \"\", there is a case\n> where following this pattern is not a good idea. If an option we\n> are eventually going to give to the underlying command takes a\n> value, the value can contain whitespace, and the option and its\n> value need to be passed as two separate arguments, it is less error\n> prone to use the \"variable only contains the value\" approach.\n>\n> Imagine that submodule--helper takes a \"--foo\" option with a greeting\n> message like \"hello world\" in such a way. We'd want to trigger it\n> this way:\n>\n> git submodule--helper --foo \"hello world\"\n>\n> as we are assuming that for some reason we need to pass them as two\n> words, and\n>\n> git submodule--helper --foo=\"hello world\"\n>\n> is not an option. In such a case, a wrapper script that takes such\n> an optional parameter in $foo is easier to write like so\n>\n> # parse\n> foo=\n> while ...\n> do\n> case \"$opt\" in\n> --foo=*) foo=\"${1#--foo=}\" ;;\n> --foo) foo=${2?\"--foo without value\"}; shift ;;\n> ...\n> esac\n> shift\n> done\n>\n> # interpolate\n> git submodule--helper ${foo:+--foo \"$foo\"}\n>\n> in order to avoid the value given to the option split at $IFS\n> whitespace. With foo='--foo=\"hello world\"', passing it to the\n> underlying command would involve use of eval and becomes error\n> prone.\n>\n> I am assuming (but I don't use \"git submodule\" very often, so my\n> assumption may be way off) that there is no such variable we need to\n> pass, but if not, we may need to reconsider and use the \"variable has\n> only value of the option\" for at least some of them.\n\nIndeed, there aren't such variables; all of the options which take\narguments have exactly one argument.\n"},{"id":"508931","messageId":"20241210184442.10723-1-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241209165009.40653-1-royeldar0@gmail.com","subject":"[PATCH v3 0/7] git-submodule.sh: improve parsing of options","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-10T18:44:35Z","receivedAt":"2024-12-10T18:45:29Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"When we run \"git submodule\", the script parses the various options and\nthen invokes \"git-submodule--helper\". Unlike most builtin git commands\nwhich parse short/long options using parse-options.c, the parsing of\narguments is completely done within git-submodule.sh; therefore, there\nare some inconsistencies with the rest of the commands, in particular\nthe parsing of option arguments given to various options.\n\nImprove the handling of option arguments for both long & short options;\nfor example, passing flags such as \"--branch=master\" or \"-j8\" now works.\n\nChanges since v2:\n\n- Remove redundant patch from the series.\n- Rename another variable for the sake of consistency.\n\nLink to v2:\n\nhttps://lore.kernel.org/git/20241209165009.40653-1-royeldar0@gmail.com\n\nRoy Eldar (7):\n git-submodule.sh: improve parsing of some long options\n git-submodule.sh: improve parsing of short options\n git-submodule.sh: get rid of isnumber\n git-submodule.sh: get rid of unused variable\n git-submodule.sh: add some comments\n git-submodule.sh: improve variables readability\n git-submodule.sh: rename some variables\n\n git-submodule.sh | 216 +++++++++++++++++++++++------------------------\n 1 file changed, 105 insertions(+), 111 deletions(-)\n\n-- \n2.30.2\n\n"},{"id":"508932","messageId":"20241210184442.10723-2-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241210184442.10723-1-royeldar0@gmail.com","subject":"[PATCH v3 1/7] git-submodule.sh: improve parsing of some long options","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-10T18:44:36Z","receivedAt":"2024-12-10T18:45:35Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"Some command-line options have a long form which takes an argument. In\nthis case, the argument can be given right after `='; for example,\n\"--depth\" takes a numerical argument, which can be given as \"--depth=X\".\n\nSupport the case where the argument is given right after `=' for all\nlong options, in order to improve consistency throughout the script.\n\nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 9 +++++++++\n 1 file changed, 9 insertions(+)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 03c5a220a2..d3e3669fde 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -77,6 +77,9 @@ cmd_add()\n \t\t\tbranch=$2\n \t\t\tshift\n \t\t\t;;\n+\t\t--branch=*)\n+\t\t\tbranch=\"${1#--branch=}\"\n+\t\t\t;;\n \t\t-f | --force)\n \t\t\tforce=$1\n \t\t\t;;\n@@ -110,6 +113,9 @@ cmd_add()\n \t\t\tcustom_name=$2\n \t\t\tshift\n \t\t\t;;\n+\t\t--name=*)\n+\t\t\tcustom_name=\"${1#--name=}\"\n+\t\t\t;;\n \t\t--depth)\n \t\t\tcase \"$2\" in '') usage ;; esac\n \t\t\tdepth=\"--depth=$2\"\n@@ -425,6 +431,9 @@ cmd_set_branch() {\n \t\t\tbranch=$2\n \t\t\tshift\n \t\t\t;;\n+\t\t--branch=*)\n+\t\t\tbranch=\"${1#--branch=}\"\n+\t\t\t;;\n \t\t--)\n \t\t\tshift\n \t\t\tbreak\n-- \n2.30.2\n\n"},{"id":"508933","messageId":"20241210184442.10723-3-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241210184442.10723-1-royeldar0@gmail.com","subject":"[PATCH v3 2/7] git-submodule.sh: improve parsing of short options","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-10T18:44:37Z","receivedAt":"2024-12-10T18:45:39Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"Some command-line options have a short form which takes an argument; for\nexample, \"--jobs\" has the form \"-j\", and it takes a numerical argument.\n\nWhen parsing short options, support the case where there is no space\nbetween the flag and the option argument, in order to improve\nconsistency with the rest of the builtin git commands.\n\nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 13 +++++++++++++\n 1 file changed, 13 insertions(+)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex d3e3669fde..fd54cb8fa6 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -77,6 +77,9 @@ cmd_add()\n \t\t\tbranch=$2\n \t\t\tshift\n \t\t\t;;\n+\t\t-b*)\n+\t\t\tbranch=\"${1#-b}\"\n+\t\t\t;;\n \t\t--branch=*)\n \t\t\tbranch=\"${1#--branch=}\"\n \t\t\t;;\n@@ -352,6 +355,9 @@ cmd_update()\n \t\t\tjobs=\"--jobs=$2\"\n \t\t\tshift\n \t\t\t;;\n+\t\t-j*)\n+\t\t\tjobs=\"--jobs=${1#-j}\"\n+\t\t\t;;\n \t\t--jobs=*)\n \t\t\tjobs=$1\n \t\t\t;;\n@@ -431,6 +437,9 @@ cmd_set_branch() {\n \t\t\tbranch=$2\n \t\t\tshift\n \t\t\t;;\n+\t\t-b*)\n+\t\t\tbranch=\"${1#-b}\"\n+\t\t\t;;\n \t\t--branch=*)\n \t\t\tbranch=\"${1#--branch=}\"\n \t\t\t;;\n@@ -519,6 +528,10 @@ cmd_summary() {\n \t\t\tisnumber \"$summary_limit\" || usage\n \t\t\tshift\n \t\t\t;;\n+\t\t-n*)\n+\t\t\tsummary_limit=\"${1#-n}\"\n+\t\t\tisnumber \"$summary_limit\" || usage\n+\t\t\t;;\n \t\t--summary-limit=*)\n \t\t\tsummary_limit=\"${1#--summary-limit=}\"\n \t\t\tisnumber \"$summary_limit\" || usage\n-- \n2.30.2\n\n"},{"id":"508934","messageId":"20241210184442.10723-4-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241210184442.10723-1-royeldar0@gmail.com","subject":"[PATCH v3 3/7] git-submodule.sh: get rid of isnumber","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-10T18:44:38Z","receivedAt":"2024-12-10T18:45:42Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"It's entirely unnecessary to check whether the argument given to an\noption (i.e. --summary-limit) is valid in the shell wrapper, since it's\nalready done when parsing the various options in git-submodule--helper.\n\nRemove this check from the script; this both improves consistency\nthroughout the script, and the error message shown to the user in case\nsome invalid non-numeric argument was passed to \"--summary-limit\" is\nmore informative as well.\n\nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 11 ++---------\n 1 file changed, 2 insertions(+), 9 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex fd54cb8fa6..3adaa8d9a3 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -53,11 +53,6 @@ jobs=\n recommend_shallow=\n filter=\n \n-isnumber()\n-{\n-\tn=$(($1 + 0)) 2>/dev/null && test \"$n\" = \"$1\"\n-}\n-\n #\n # Add a new submodule to the working tree, .gitmodules and the index\n #\n@@ -524,17 +519,15 @@ cmd_summary() {\n \t\t\tfor_status=\"$1\"\n \t\t\t;;\n \t\t-n|--summary-limit)\n+\t\t\tcase \"$2\" in '') usage ;; esac\n \t\t\tsummary_limit=\"$2\"\n-\t\t\tisnumber \"$summary_limit\" || usage\n \t\t\tshift\n \t\t\t;;\n \t\t-n*)\n \t\t\tsummary_limit=\"${1#-n}\"\n-\t\t\tisnumber \"$summary_limit\" || usage\n \t\t\t;;\n \t\t--summary-limit=*)\n \t\t\tsummary_limit=\"${1#--summary-limit=}\"\n-\t\t\tisnumber \"$summary_limit\" || usage\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -554,7 +547,7 @@ cmd_summary() {\n \t\t${files:+--files} \\\n \t\t${cached:+--cached} \\\n \t\t${for_status:+--for-status} \\\n-\t\t${summary_limit:+-n $summary_limit} \\\n+\t\t${summary_limit:+-n \"$summary_limit\"} \\\n \t\t-- \\\n \t\t\"$@\"\n }\n-- \n2.30.2\n\n"},{"id":"508935","messageId":"20241210184442.10723-5-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241210184442.10723-1-royeldar0@gmail.com","subject":"[PATCH v3 4/7] git-submodule.sh: get rid of unused variable","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-10T18:44:39Z","receivedAt":"2024-12-10T18:45:44Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"Remove the variable \"$diff_cmd\" which is no longer used.\n\nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 3adaa8d9a3..ba3bef8821 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -503,7 +503,6 @@ cmd_set_url() {\n cmd_summary() {\n \tsummary_limit=-1\n \tfor_status=\n-\tdiff_cmd=diff-index\n \n \t# parse $args after \"submodule ... summary\".\n \twhile test $# -ne 0\n-- \n2.30.2\n\n"},{"id":"508936","messageId":"20241210184442.10723-6-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241210184442.10723-1-royeldar0@gmail.com","subject":"[PATCH v3 5/7] git-submodule.sh: add some comments","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-10T18:44:40Z","receivedAt":"2024-12-10T18:45:46Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"Add a couple of comments in a few functions where they were missing.\n\nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex ba3bef8821..ee86e4de46 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -418,6 +418,7 @@ cmd_set_branch() {\n \tdefault=\n \tbranch=\n \n+\t# parse $args after \"submodule ... set-branch\".\n \twhile test $# -ne 0\n \tdo\n \t\tcase \"$1\" in\n@@ -466,6 +467,7 @@ cmd_set_branch() {\n # $@ = requested path, requested url\n #\n cmd_set_url() {\n+\t# parse $args after \"submodule ... set-url\".\n \twhile test $# -ne 0\n \tdo\n \t\tcase \"$1\" in\n@@ -604,6 +606,7 @@ cmd_status()\n #\n cmd_sync()\n {\n+\t# parse $args after \"submodule ... sync\".\n \twhile test $# -ne 0\n \tdo\n \t\tcase \"$1\" in\n-- \n2.30.2\n\n"},{"id":"508937","messageId":"20241210184442.10723-7-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241210184442.10723-1-royeldar0@gmail.com","subject":"[PATCH v3 6/7] git-submodule.sh: improve variables readability","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-10T18:44:41Z","receivedAt":"2024-12-10T18:45:49Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"When git-submodule.sh parses various options and switches, it sets some\nvariables to values; the variables in turn affect the options given to\ngit-submodule--helper.\n\nCurrently, variables which correspond to switches have boolean values\n(for example, whenever \"--force\" is passed, force=1), while variables\nwhich correspond to options which take arguments have string values that\nsometimes contain the option name and sometimes only the option value.\n\nSet all of the variables to strings which contain the option name (e.g.\nforce=\"--force\" rather than force=1); this has a couple of advantages:\nit improves consistency, readability and debuggability.\n\nSuggested-by: Junio C Hamano \nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 213 +++++++++++++++++++++--------------------------\n 1 file changed, 95 insertions(+), 118 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex ee86e4de46..2da4d55d64 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -52,6 +52,10 @@ single_branch=\n jobs=\n recommend_shallow=\n filter=\n+deinit_all=\n+default=\n+summary_limit=\n+for_status=\n \n #\n # Add a new submodule to the working tree, .gitmodules and the index\n@@ -63,37 +67,33 @@ filter=\n cmd_add()\n {\n \t# parse $args after \"submodule ... add\".\n-\treference_path=\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\tbranch=\"--branch=$2\"\n \t\t\tshift\n \t\t\t;;\n-\t\t-b*)\n-\t\t\tbranch=\"${1#-b}\"\n-\t\t\t;;\n-\t\t--branch=*)\n-\t\t\tbranch=\"${1#--branch=}\"\n+\t\t-b* | --branch*)\n+\t\t\tbranch=\"$1\"\n \t\t\t;;\n \t\t-f | --force)\n \t\t\tforce=$1\n \t\t\t;;\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\t;;\n \t\t--progress)\n-\t\t\tprogress=1\n+\t\t\tprogress=$1\n \t\t\t;;\n \t\t--reference)\n \t\t\tcase \"$2\" in '') usage ;; esac\n-\t\t\treference_path=$2\n+\t\t\treference=\"--reference=$2\"\n \t\t\tshift\n \t\t\t;;\n \t\t--reference=*)\n-\t\t\treference_path=\"${1#--reference=}\"\n+\t\t\treference=\"$1\"\n \t\t\t;;\n \t\t--ref-format)\n \t\t\tcase \"$2\" in '') usage ;; esac\n@@ -104,15 +104,15 @@ cmd_add()\n \t\t\tref_format=\"$1\"\n \t\t\t;;\n \t\t--dissociate)\n-\t\t\tdissociate=1\n+\t\t\tdissociate=$1\n \t\t\t;;\n \t\t--name)\n \t\t\tcase \"$2\" in '') usage ;; esac\n-\t\t\tcustom_name=$2\n+\t\t\tcustom_name=\"--name=$2\"\n \t\t\tshift\n \t\t\t;;\n \t\t--name=*)\n-\t\t\tcustom_name=\"${1#--name=}\"\n+\t\t\tcustom_name=\"$1\"\n \t\t\t;;\n \t\t--depth)\n \t\t\tcase \"$2\" in '') usage ;; esac\n@@ -120,7 +120,7 @@ cmd_add()\n \t\t\tshift\n \t\t\t;;\n \t\t--depth=*)\n-\t\t\tdepth=$1\n+\t\t\tdepth=\"$1\"\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -142,14 +142,14 @@ cmd_add()\n \tfi\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper add \\\n-\t\t${quiet:+--quiet} \\\n-\t\t${force:+--force} \\\n-\t\t${progress:+\"--progress\"} \\\n-\t\t${branch:+--branch \"$branch\"} \\\n-\t\t${reference_path:+--reference \"$reference_path\"} \\\n+\t\t$quiet \\\n+\t\t$force \\\n+\t\t$progress \\\n+\t\t${branch:+\"$branch\"} \\\n+\t\t${reference:+\"$reference\"} \\\n \t\t${ref_format:+\"$ref_format\"} \\\n-\t\t${dissociate:+--dissociate} \\\n-\t\t${custom_name:+--name \"$custom_name\"} \\\n+\t\t$dissociate \\\n+\t\t${custom_name:+\"$custom_name\"} \\\n \t\t${depth:+\"$depth\"} \\\n \t\t-- \\\n \t\t\"$@\"\n@@ -168,10 +168,10 @@ cmd_foreach()\n \tdo\n \t\tcase \"$1\" in\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\t;;\n \t\t--recursive)\n-\t\t\trecursive=1\n+\t\t\trecursive=$1\n \t\t\t;;\n \t\t-*)\n \t\t\tusage\n@@ -184,8 +184,8 @@ cmd_foreach()\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper foreach \\\n-\t\t${quiet:+--quiet} \\\n-\t\t${recursive:+--recursive} \\\n+\t\t$quiet \\\n+\t\t$recursive \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -202,7 +202,7 @@ cmd_init()\n \tdo\n \t\tcase \"$1\" in\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -219,7 +219,7 @@ cmd_init()\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper init \\\n-\t\t${quiet:+--quiet} \\\n+\t\t$quiet \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -230,7 +230,6 @@ cmd_init()\n cmd_deinit()\n {\n \t# parse $args after \"submodule ... deinit\".\n-\tdeinit_all=\n \twhile test $# -ne 0\n \tdo\n \t\tcase \"$1\" in\n@@ -238,10 +237,10 @@ cmd_deinit()\n \t\t\tforce=$1\n \t\t\t;;\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\t;;\n \t\t--all)\n-\t\t\tdeinit_all=t\n+\t\t\tdeinit_all=$1\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -258,9 +257,9 @@ cmd_deinit()\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper deinit \\\n-\t\t${quiet:+--quiet} \\\n-\t\t${force:+--force} \\\n-\t\t${deinit_all:+--all} \\\n+\t\t$quiet \\\n+\t\t$force \\\n+\t\t$deinit_all \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -277,31 +276,31 @@ cmd_update()\n \tdo\n \t\tcase \"$1\" in\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\t;;\n \t\t-v|--verbose)\n-\t\t\tquiet=0\n+\t\t\tquiet=\n \t\t\t;;\n \t\t--progress)\n-\t\t\tprogress=1\n+\t\t\tprogress=$1\n \t\t\t;;\n \t\t-i|--init)\n-\t\t\tinit=1\n+\t\t\tinit=$1\n \t\t\t;;\n \t\t--require-init)\n-\t\t\trequire_init=1\n+\t\t\trequire_init=$1\n \t\t\t;;\n \t\t--remote)\n-\t\t\tremote=1\n+\t\t\tremote=$1\n \t\t\t;;\n \t\t-N|--no-fetch)\n-\t\t\tnofetch=1\n+\t\t\tnofetch=$1\n \t\t\t;;\n \t\t-f|--force)\n \t\t\tforce=$1\n \t\t\t;;\n \t\t-r|--rebase)\n-\t\t\trebase=1\n+\t\t\trebase=$1\n \t\t\t;;\n \t\t--ref-format)\n \t\t\tcase \"$2\" in '') usage ;; esac\n@@ -320,22 +319,19 @@ cmd_update()\n \t\t\treference=\"$1\"\n \t\t\t;;\n \t\t--dissociate)\n-\t\t\tdissociate=1\n+\t\t\tdissociate=$1\n \t\t\t;;\n \t\t-m|--merge)\n-\t\t\tmerge=1\n+\t\t\tmerge=$1\n \t\t\t;;\n \t\t--recursive)\n-\t\t\trecursive=1\n+\t\t\trecursive=$1\n \t\t\t;;\n \t\t--checkout)\n-\t\t\tcheckout=1\n-\t\t\t;;\n-\t\t--recommend-shallow)\n-\t\t\trecommend_shallow=\"--recommend-shallow\"\n+\t\t\tcheckout=$1\n \t\t\t;;\n-\t\t--no-recommend-shallow)\n-\t\t\trecommend_shallow=\"--no-recommend-shallow\"\n+\t\t--recommend-shallow|--no-recommend-shallow)\n+\t\t\trecommend_shallow=$1\n \t\t\t;;\n \t\t--depth)\n \t\t\tcase \"$2\" in '') usage ;; esac\n@@ -343,24 +339,18 @@ cmd_update()\n \t\t\tshift\n \t\t\t;;\n \t\t--depth=*)\n-\t\t\tdepth=$1\n+\t\t\tdepth=\"$1\"\n \t\t\t;;\n \t\t-j|--jobs)\n \t\t\tcase \"$2\" in '') usage ;; esac\n \t\t\tjobs=\"--jobs=$2\"\n \t\t\tshift\n \t\t\t;;\n-\t\t-j*)\n-\t\t\tjobs=\"--jobs=${1#-j}\"\n-\t\t\t;;\n-\t\t--jobs=*)\n-\t\t\tjobs=$1\n+\t\t-j*|--jobs*)\n+\t\t\tjobs=\"$1\"\n \t\t\t;;\n-\t\t--single-branch)\n-\t\t\tsingle_branch=\"--single-branch\"\n-\t\t\t;;\n-\t\t--no-single-branch)\n-\t\t\tsingle_branch=\"--no-single-branch\"\n+\t\t--single-branch|--no-single-branch)\n+\t\t\tsingle_branch=$1\n \t\t\t;;\n \t\t--filter)\n \t\t\tcase \"$2\" in '') usage ;; esac\n@@ -385,22 +375,21 @@ cmd_update()\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper update \\\n-\t\t${quiet:+--quiet} \\\n-\t\t${force:+--force} \\\n-\t\t${progress:+\"--progress\"} \\\n-\t\t${remote:+--remote} \\\n-\t\t${recursive:+--recursive} \\\n-\t\t${init:+--init} \\\n-\t\t${nofetch:+--no-fetch} \\\n-\t\t${rebase:+--rebase} \\\n-\t\t${merge:+--merge} \\\n-\t\t${checkout:+--checkout} \\\n+\t\t$quiet \\\n+\t\t$force \\\n+\t\t$progress \\\n+\t\t$remote \\\n+\t\t$recursive \\\n+\t\t$init \\\n+\t\t$nofetch \\\n+\t\t$rebase \\\n+\t\t$merge \\\n+\t\t$checkout \\\n \t\t${ref_format:+\"$ref_format\"} \\\n \t\t${reference:+\"$reference\"} \\\n-\t\t${dissociate:+\"--dissociate\"} \\\n+\t\t$dissociate \\\n \t\t${depth:+\"$depth\"} \\\n-\t\t${require_init:+--require-init} \\\n-\t\t${dissociate:+\"--dissociate\"} \\\n+\t\t$require_init \\\n \t\t$single_branch \\\n \t\t$recommend_shallow \\\n \t\t$jobs \\\n@@ -415,9 +404,6 @@ cmd_update()\n # $@ = requested path\n #\n cmd_set_branch() {\n-\tdefault=\n-\tbranch=\n-\n \t# parse $args after \"submodule ... set-branch\".\n \twhile test $# -ne 0\n \tdo\n@@ -426,18 +412,15 @@ cmd_set_branch() {\n \t\t\t# we don't do anything with this but we need to accept it\n \t\t\t;;\n \t\t-d|--default)\n-\t\t\tdefault=1\n+\t\t\tdefault=$1\n \t\t\t;;\n \t\t-b|--branch)\n \t\t\tcase \"$2\" in '') usage ;; esac\n-\t\t\tbranch=$2\n+\t\t\tbranch=\"--branch=$2\"\n \t\t\tshift\n \t\t\t;;\n-\t\t-b*)\n-\t\t\tbranch=\"${1#-b}\"\n-\t\t\t;;\n-\t\t--branch=*)\n-\t\t\tbranch=\"${1#--branch=}\"\n+\t\t-b*|--branch=*)\n+\t\t\tbranch=\"$1\"\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -454,9 +437,9 @@ cmd_set_branch() {\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper set-branch \\\n-\t\t${quiet:+--quiet} \\\n-\t\t${branch:+--branch \"$branch\"} \\\n-\t\t${default:+--default} \\\n+\t\t$quiet \\\n+\t\t${branch:+\"$branch\"} \\\n+\t\t$default \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -472,7 +455,7 @@ cmd_set_url() {\n \tdo\n \t\tcase \"$1\" in\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -489,7 +472,7 @@ cmd_set_url() {\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper set-url \\\n-\t\t${quiet:+--quiet} \\\n+\t\t$quiet \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -503,32 +486,26 @@ cmd_set_url() {\n # $@ = [commit (default 'HEAD'),] requested paths (default all)\n #\n cmd_summary() {\n-\tsummary_limit=-1\n-\tfor_status=\n-\n \t# parse $args after \"submodule ... summary\".\n \twhile test $# -ne 0\n \tdo\n \t\tcase \"$1\" in\n \t\t--cached)\n-\t\t\tcached=1\n+\t\t\tcached=$1\n \t\t\t;;\n \t\t--files)\n-\t\t\tfiles=\"$1\"\n+\t\t\tfiles=$1\n \t\t\t;;\n \t\t--for-status)\n-\t\t\tfor_status=\"$1\"\n+\t\t\tfor_status=$1\n \t\t\t;;\n \t\t-n|--summary-limit)\n \t\t\tcase \"$2\" in '') usage ;; esac\n-\t\t\tsummary_limit=\"$2\"\n+\t\t\tsummary_limit=\"--summary-limit=$2\"\n \t\t\tshift\n \t\t\t;;\n-\t\t-n*)\n-\t\t\tsummary_limit=\"${1#-n}\"\n-\t\t\t;;\n-\t\t--summary-limit=*)\n-\t\t\tsummary_limit=\"${1#--summary-limit=}\"\n+\t\t-n*|--summary-limit=*)\n+\t\t\tsummary_limit=\"$1\"\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -545,10 +522,10 @@ cmd_summary() {\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper summary \\\n-\t\t${files:+--files} \\\n-\t\t${cached:+--cached} \\\n-\t\t${for_status:+--for-status} \\\n-\t\t${summary_limit:+-n \"$summary_limit\"} \\\n+\t\t$files \\\n+\t\t$cached \\\n+\t\t$for_status \\\n+\t\t${summary_limit:+\"$summary_limit\"} \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -569,13 +546,13 @@ cmd_status()\n \tdo\n \t\tcase \"$1\" in\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\t;;\n \t\t--cached)\n-\t\t\tcached=1\n+\t\t\tcached=$1\n \t\t\t;;\n \t\t--recursive)\n-\t\t\trecursive=1\n+\t\t\trecursive=$1\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -592,9 +569,9 @@ cmd_status()\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper status \\\n-\t\t${quiet:+--quiet} \\\n-\t\t${cached:+--cached} \\\n-\t\t${recursive:+--recursive} \\\n+\t\t$quiet \\\n+\t\t$cached \\\n+\t\t$recursive \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -611,11 +588,11 @@ cmd_sync()\n \tdo\n \t\tcase \"$1\" in\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\tshift\n \t\t\t;;\n \t\t--recursive)\n-\t\t\trecursive=1\n+\t\t\trecursive=$1\n \t\t\tshift\n \t\t\t;;\n \t\t--)\n@@ -632,8 +609,8 @@ cmd_sync()\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper sync \\\n-\t\t${quiet:+--quiet} \\\n-\t\t${recursive:+--recursive} \\\n+\t\t$quiet \\\n+\t\t$recursive \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -656,10 +633,10 @@ do\n \t\tcommand=$1\n \t\t;;\n \t-q|--quiet)\n-\t\tquiet=1\n+\t\tquiet=$1\n \t\t;;\n \t--cached)\n-\t\tcached=1\n+\t\tcached=$1\n \t\t;;\n \t--)\n \t\tbreak\n-- \n2.30.2\n\n"},{"id":"508938","messageId":"20241210184442.10723-8-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241210184442.10723-1-royeldar0@gmail.com","subject":"[PATCH v3 7/7] git-submodule.sh: rename some variables","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-10T18:44:42Z","receivedAt":"2024-12-10T18:45:51Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"Every switch and option which is passed to git-submodule.sh has a\ncorresponding variable which is set accordingly; by convention, the name\nof the variable is the option name (for example, \"--jobs\" and \"$jobs\").\n\nRename \"$custom_name\", \"$deinit_all\" and \"$nofetch\", for consistency.\n\nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 2da4d55d64..24276847ad 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -40,11 +40,11 @@ init=\n require_init=\n files=\n remote=\n-nofetch=\n+no_fetch=\n rebase=\n merge=\n checkout=\n-custom_name=\n+name=\n depth=\n progress=\n dissociate=\n@@ -52,7 +52,7 @@ single_branch=\n jobs=\n recommend_shallow=\n filter=\n-deinit_all=\n+all=\n default=\n summary_limit=\n for_status=\n@@ -108,11 +108,11 @@ cmd_add()\n \t\t\t;;\n \t\t--name)\n \t\t\tcase \"$2\" in '') usage ;; esac\n-\t\t\tcustom_name=\"--name=$2\"\n+\t\t\tname=\"--name=$2\"\n \t\t\tshift\n \t\t\t;;\n \t\t--name=*)\n-\t\t\tcustom_name=\"$1\"\n+\t\t\tname=\"$1\"\n \t\t\t;;\n \t\t--depth)\n \t\t\tcase \"$2\" in '') usage ;; esac\n@@ -149,7 +149,7 @@ cmd_add()\n \t\t${reference:+\"$reference\"} \\\n \t\t${ref_format:+\"$ref_format\"} \\\n \t\t$dissociate \\\n-\t\t${custom_name:+\"$custom_name\"} \\\n+\t\t${name:+\"$name\"} \\\n \t\t${depth:+\"$depth\"} \\\n \t\t-- \\\n \t\t\"$@\"\n@@ -240,7 +240,7 @@ cmd_deinit()\n \t\t\tquiet=$1\n \t\t\t;;\n \t\t--all)\n-\t\t\tdeinit_all=$1\n+\t\t\tall=$1\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -259,7 +259,7 @@ cmd_deinit()\n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper deinit \\\n \t\t$quiet \\\n \t\t$force \\\n-\t\t$deinit_all \\\n+\t\t$all \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -294,7 +294,7 @@ cmd_update()\n \t\t\tremote=$1\n \t\t\t;;\n \t\t-N|--no-fetch)\n-\t\t\tnofetch=$1\n+\t\t\tno_fetch=$1\n \t\t\t;;\n \t\t-f|--force)\n \t\t\tforce=$1\n@@ -381,7 +381,7 @@ cmd_update()\n \t\t$remote \\\n \t\t$recursive \\\n \t\t$init \\\n-\t\t$nofetch \\\n+\t\t$no_fetch \\\n \t\t$rebase \\\n \t\t$merge \\\n \t\t$checkout \\\n-- \n2.30.2\n\n"},{"id":"508940","messageId":"xmqqikrrjdds.fsf@gitster.g","threadId":"62611","inReplyTo":"CAOfFamkEmJgUUj8C_TGbjgKs35vSBzNCDpLvK+utqnTtxd5iHQ@mail.gmail.com","subject":"Re: [PATCH v2 0/8] git-submodule.sh: improve parsing of options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-11T00:02:07Z","receivedAt":"2024-12-11T00:02:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Roy E writes:\n\n>> I am assuming (but I don't use \"git submodule\" very often, so my\n>> assumption may be way off) that there is no such variable we need to\n>> pass, but if not, we may need to reconsider and use the \"variable has\n>> only value of the option\" for at least some of them.\n>\n> Indeed, there aren't such variables; all of the options which take\n> arguments have exactly one argument.\n\nJust to make sure we are on the same page,\n\n --foo \"hello world\"\n\nis an example of an option \"foo\" that takes exactly one argument, a\nstring which happens to have a whitespace in it, and is an example\nfor which \"variable has the dash-dash option, equals, and its value\"\npattern would not work well.\n\nIf we pass an argument that is an end-user provided message or is a\nproject controlled pathname, we are likely to be in the same\nsituation.\n\nIf we can pass it as\n\n --foo=\"hello world\"\n\nthen we are safe, as we can do\n\n\tfoo=\"--foo=hello world\"\n\t... later ...\n\tgit cmd ${foo:+\"$foo\"}\n"},{"id":"508941","messageId":"xmqq8qsnjcsy.fsf@gitster.g","threadId":"62611","inReplyTo":"20241210184442.10723-7-royeldar0@gmail.com","subject":"Re: [PATCH v3 6/7] git-submodule.sh: improve variables readability","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-11T00:14:37Z","receivedAt":"2024-12-11T00:14:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Roy Eldar writes:\n\n> +\t\t-b* | --branch*)\n> +\t\t\tbranch=\"$1\"\n\nOnce another option like \"--branches-only\" will be caught by this\ncase arm. Do not omit \"=\" in the pattern.\n\n> @@ -142,14 +142,14 @@ cmd_add()\n> \tfi\n> \n> \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper add \\\n> -\t\t${quiet:+--quiet} \\\n> -\t\t${force:+--force} \\\n> -\t\t${progress:+\"--progress\"} \\\n> -\t\t${branch:+--branch \"$branch\"} \\\n> -\t\t${reference_path:+--reference \"$reference_path\"} \\\n> +\t\t$quiet \\\n> +\t\t$force \\\n> +\t\t$progress \\\n> +\t\t${branch:+\"$branch\"} \\\n> +\t\t${reference:+\"$reference\"} \\\n> \t\t${ref_format:+\"$ref_format\"} \\\n> -\t\t${dissociate:+--dissociate} \\\n> -\t\t${custom_name:+--name \"$custom_name\"} \\\n> +\t\t$dissociate \\\n> +\t\t${custom_name:+\"$custom_name\"} \\\n> \t\t${depth:+\"$depth\"} \\\n> \t\t-- \\\n> \t\t\"$@\"\n\nLooking good.\n"},{"id":"508942","messageId":"Z1jxSJKl8nwGbYa5@danh.dev","threadId":"62611","inReplyTo":"20241210184442.10723-7-royeldar0@gmail.com","subject":"Re: [PATCH v3 6/7] git-submodule.sh: improve variables readability","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2024-12-11T01:56:24Z","receivedAt":"2024-12-11T01:56:28Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2024-12-10 20:44:41+0200, Roy Eldar wrote:\n> \t\t\t;;\n> \t\t--reference)\n> \t\t\tcase \"$2\" in '') usage ;; esac\n> -\t\t\treference_path=$2\n> +\t\t\treference=\"--reference=$2\"\n> \t\t\tshift\n> \t\t\t;;\n> \t\t--reference=*)\n> -\t\t\treference_path=\"${1#--reference=}\"\n> +\t\t\treference=\"$1\"\n\n--reference takes a path to some repository,\n(see also git-clone --reference),\nthus it can have any characters, including but not limit to whitespace.\nI think we need to discard this hunk!\n\n-- \nDanh\n"},{"id":"508943","messageId":"xmqq4j3akayp.fsf@gitster.g","threadId":"62611","inReplyTo":"Z1jxSJKl8nwGbYa5@danh.dev","subject":"Re: [PATCH v3 6/7] git-submodule.sh: improve variables readability","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-11T06:09:02Z","receivedAt":"2024-12-11T06:09:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh writes:\n\n>> \t\t--reference=*)\n>> -\t\t\treference_path=\"${1#--reference=}\"\n>> +\t\t\treference=\"$1\"\n>\n> --reference takes a path to some repository,\n> (see also git-clone --reference),\n> thus it can have any characters, including but not limit to whitespace.\n> I think we need to discard this hunk!\n\nI didn't double check the code that uses the variable, but as long\nas the code that uses $reference writs it correctly, e.g.\n\n git subcmd ${reference:+\"$reference\"} ...\n\nit should correctly pass what it received in the above assignment\nfrom the user in $1 just fine, even if its value can contain any\narbitrary byte, I would think.\n\n\n"},{"id":"508944","messageId":"CAOfFam=_G=EPkw-fCQD___gFc3U7rwnVr_uteKG9-USK8=veRA@mail.gmail.com","threadId":"62611","inReplyTo":"xmqqikrrjdds.fsf@gitster.g","subject":"Re: [PATCH v2 0/8] git-submodule.sh: improve parsing of options","fromName":"Roy E","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-11T06:13:38Z","receivedAt":"2024-12-11T06:13:49Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"Junio C Hamano wrote:\n\n> Just to make sure we are on the same page,\n>\n> --foo \"hello world\"\n>\n> is an example of an option \"foo\" that takes exactly one argument, a\n> string which happens to have a whitespace in it, and is an example\n> for which \"variable has the dash-dash option, equals, and its value\"\n> pattern would not work well.\n\nI'm not sure why then this pattern would not work; when the argument is\npassed to the option in this case, we set the variable to \"--foo=$2\",\nso it should be fine (like you've written below).\n\n> If we can pass it as\n>\n> --foo=\"hello world\"\n>\n> then we are safe, as we can do\n>\n> foo=\"--foo=hello world\"\n> ... later ...\n> git cmd ${foo:+\"$foo\"}\n\nAll of the options with arguments of git-submodule--helper can be\npassed as \"--foo=...\", and spaces (or other misc characters) that\nappear in the option value shouldn't pose any problem whatsoever;\nthe logic in parse-options.c::parse_long_opt confirms that.\n"},{"id":"508946","messageId":"xmqqzfl2iw2b.fsf@gitster.g","threadId":"62611","inReplyTo":"CAOfFam=_G=EPkw-fCQD___gFc3U7rwnVr_uteKG9-USK8=veRA@mail.gmail.com","subject":"Re: [PATCH v2 0/8] git-submodule.sh: improve parsing of options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-11T06:16:12Z","receivedAt":"2024-12-11T06:16:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Roy E writes:\n\n> All of the options with arguments of git-submodule--helper can be\n> passed as \"--foo=...\",\n\nThat part was what I was cautioning about. I suspect it is true as\nit should be using parse-options API but didn't double check myself.\n\n> and spaces (or other misc characters) that\n> appear in the option value shouldn't pose any problem whatsoever;\n\nThe latter is consistent with what I gave you.\n\nThanks.\n\n"},{"id":"508947","messageId":"CAOfFamn7cxebqr56UPCBU7z+bA85Lws8KSUWBDDah9ORmqLE=w@mail.gmail.com","threadId":"62611","inReplyTo":"xmqq8qsnjcsy.fsf@gitster.g","subject":"Re: [PATCH v3 6/7] git-submodule.sh: improve variables readability","fromName":"Roy E","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-11T06:21:38Z","receivedAt":"2024-12-11T06:21:49Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"Junio C Hamano wrote:\n\n> Roy Eldar writes:\n>\n> > + -b* | --branch*)\n> > + branch=\"$1\"\n>\n> Once another option like \"--branches-only\" will be caught by this\n> case arm. Do not omit \"=\" in the pattern.\n\nThanks. I noticed that I did so in a couple of other places as well, so\nI'll fix that and send a v4 patch series.\n"},{"id":"508948","messageId":"20241211063234.7610-1-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241210184442.10723-1-royeldar0@gmail.com","subject":"[PATCH v4 0/7] git-submodule.sh: improve parsing of options","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-11T06:32:27Z","receivedAt":"2024-12-11T06:33:37Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"When we run \"git submodule\", the script parses the various options and\nthen invokes \"git-submodule--helper\". Unlike most builtin git commands\nwhich parse short/long options using parse-options.c, the parsing of\narguments is completely done within git-submodule.sh; therefore, there\nare some inconsistencies with the rest of the commands, in particular\nthe parsing of option arguments given to various options.\n\nImprove the handling of option arguments for both long & short options;\nfor example, passing flags such as \"--branch=master\" or \"-j8\" now works.\n\nChanges since v3:\n\n- Minor typos in some switch case patterns.\n\nLink to v3:\n\nhttps://lore.kernel.org/git/20241210184442.10723-1-royeldar0@gmail.com\n\nRoy Eldar (7):\n git-submodule.sh: improve parsing of some long options\n git-submodule.sh: improve parsing of short options\n git-submodule.sh: get rid of isnumber\n git-submodule.sh: get rid of unused variable\n git-submodule.sh: add some comments\n git-submodule.sh: improve variables readability\n git-submodule.sh: rename some variables\n\n git-submodule.sh | 216 +++++++++++++++++++++++------------------------\n 1 file changed, 105 insertions(+), 111 deletions(-)\n\n-- \n2.30.2\n\n"},{"id":"508949","messageId":"20241211063234.7610-2-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241211063234.7610-1-royeldar0@gmail.com","subject":"[PATCH v4 1/7] git-submodule.sh: improve parsing of some long options","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-11T06:32:28Z","receivedAt":"2024-12-11T06:33:41Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"Some command-line options have a long form which takes an argument. In\nthis case, the argument can be given right after `='; for example,\n\"--depth\" takes a numerical argument, which can be given as \"--depth=X\".\n\nSupport the case where the argument is given right after `=' for all\nlong options, in order to improve consistency throughout the script.\n\nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 9 +++++++++\n 1 file changed, 9 insertions(+)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 03c5a220a2..d3e3669fde 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -77,6 +77,9 @@ cmd_add()\n \t\t\tbranch=$2\n \t\t\tshift\n \t\t\t;;\n+\t\t--branch=*)\n+\t\t\tbranch=\"${1#--branch=}\"\n+\t\t\t;;\n \t\t-f | --force)\n \t\t\tforce=$1\n \t\t\t;;\n@@ -110,6 +113,9 @@ cmd_add()\n \t\t\tcustom_name=$2\n \t\t\tshift\n \t\t\t;;\n+\t\t--name=*)\n+\t\t\tcustom_name=\"${1#--name=}\"\n+\t\t\t;;\n \t\t--depth)\n \t\t\tcase \"$2\" in '') usage ;; esac\n \t\t\tdepth=\"--depth=$2\"\n@@ -425,6 +431,9 @@ cmd_set_branch() {\n \t\t\tbranch=$2\n \t\t\tshift\n \t\t\t;;\n+\t\t--branch=*)\n+\t\t\tbranch=\"${1#--branch=}\"\n+\t\t\t;;\n \t\t--)\n \t\t\tshift\n \t\t\tbreak\n-- \n2.30.2\n\n"},{"id":"508950","messageId":"20241211063234.7610-3-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241211063234.7610-1-royeldar0@gmail.com","subject":"[PATCH v4 2/7] git-submodule.sh: improve parsing of short options","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-11T06:32:29Z","receivedAt":"2024-12-11T06:33:45Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"Some command-line options have a short form which takes an argument; for\nexample, \"--jobs\" has the form \"-j\", and it takes a numerical argument.\n\nWhen parsing short options, support the case where there is no space\nbetween the flag and the option argument, in order to improve\nconsistency with the rest of the builtin git commands.\n\nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 13 +++++++++++++\n 1 file changed, 13 insertions(+)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex d3e3669fde..fd54cb8fa6 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -77,6 +77,9 @@ cmd_add()\n \t\t\tbranch=$2\n \t\t\tshift\n \t\t\t;;\n+\t\t-b*)\n+\t\t\tbranch=\"${1#-b}\"\n+\t\t\t;;\n \t\t--branch=*)\n \t\t\tbranch=\"${1#--branch=}\"\n \t\t\t;;\n@@ -352,6 +355,9 @@ cmd_update()\n \t\t\tjobs=\"--jobs=$2\"\n \t\t\tshift\n \t\t\t;;\n+\t\t-j*)\n+\t\t\tjobs=\"--jobs=${1#-j}\"\n+\t\t\t;;\n \t\t--jobs=*)\n \t\t\tjobs=$1\n \t\t\t;;\n@@ -431,6 +437,9 @@ cmd_set_branch() {\n \t\t\tbranch=$2\n \t\t\tshift\n \t\t\t;;\n+\t\t-b*)\n+\t\t\tbranch=\"${1#-b}\"\n+\t\t\t;;\n \t\t--branch=*)\n \t\t\tbranch=\"${1#--branch=}\"\n \t\t\t;;\n@@ -519,6 +528,10 @@ cmd_summary() {\n \t\t\tisnumber \"$summary_limit\" || usage\n \t\t\tshift\n \t\t\t;;\n+\t\t-n*)\n+\t\t\tsummary_limit=\"${1#-n}\"\n+\t\t\tisnumber \"$summary_limit\" || usage\n+\t\t\t;;\n \t\t--summary-limit=*)\n \t\t\tsummary_limit=\"${1#--summary-limit=}\"\n \t\t\tisnumber \"$summary_limit\" || usage\n-- \n2.30.2\n\n"},{"id":"508951","messageId":"20241211063234.7610-4-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241211063234.7610-1-royeldar0@gmail.com","subject":"[PATCH v4 3/7] git-submodule.sh: get rid of isnumber","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-11T06:32:30Z","receivedAt":"2024-12-11T06:33:48Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"It's entirely unnecessary to check whether the argument given to an\noption (i.e. --summary-limit) is valid in the shell wrapper, since it's\nalready done when parsing the various options in git-submodule--helper.\n\nRemove this check from the script; this both improves consistency\nthroughout the script, and the error message shown to the user in case\nsome invalid non-numeric argument was passed to \"--summary-limit\" is\nmore informative as well.\n\nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 11 ++---------\n 1 file changed, 2 insertions(+), 9 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex fd54cb8fa6..3adaa8d9a3 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -53,11 +53,6 @@ jobs=\n recommend_shallow=\n filter=\n \n-isnumber()\n-{\n-\tn=$(($1 + 0)) 2>/dev/null && test \"$n\" = \"$1\"\n-}\n-\n #\n # Add a new submodule to the working tree, .gitmodules and the index\n #\n@@ -524,17 +519,15 @@ cmd_summary() {\n \t\t\tfor_status=\"$1\"\n \t\t\t;;\n \t\t-n|--summary-limit)\n+\t\t\tcase \"$2\" in '') usage ;; esac\n \t\t\tsummary_limit=\"$2\"\n-\t\t\tisnumber \"$summary_limit\" || usage\n \t\t\tshift\n \t\t\t;;\n \t\t-n*)\n \t\t\tsummary_limit=\"${1#-n}\"\n-\t\t\tisnumber \"$summary_limit\" || usage\n \t\t\t;;\n \t\t--summary-limit=*)\n \t\t\tsummary_limit=\"${1#--summary-limit=}\"\n-\t\t\tisnumber \"$summary_limit\" || usage\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -554,7 +547,7 @@ cmd_summary() {\n \t\t${files:+--files} \\\n \t\t${cached:+--cached} \\\n \t\t${for_status:+--for-status} \\\n-\t\t${summary_limit:+-n $summary_limit} \\\n+\t\t${summary_limit:+-n \"$summary_limit\"} \\\n \t\t-- \\\n \t\t\"$@\"\n }\n-- \n2.30.2\n\n"},{"id":"508952","messageId":"20241211063234.7610-5-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241211063234.7610-1-royeldar0@gmail.com","subject":"[PATCH v4 4/7] git-submodule.sh: get rid of unused variable","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-11T06:32:31Z","receivedAt":"2024-12-11T06:33:52Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"Remove the variable \"$diff_cmd\" which is no longer used.\n\nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 3adaa8d9a3..ba3bef8821 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -503,7 +503,6 @@ cmd_set_url() {\n cmd_summary() {\n \tsummary_limit=-1\n \tfor_status=\n-\tdiff_cmd=diff-index\n \n \t# parse $args after \"submodule ... summary\".\n \twhile test $# -ne 0\n-- \n2.30.2\n\n"},{"id":"508953","messageId":"20241211063234.7610-6-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241211063234.7610-1-royeldar0@gmail.com","subject":"[PATCH v4 5/7] git-submodule.sh: add some comments","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-11T06:32:32Z","receivedAt":"2024-12-11T06:33:53Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"Add a couple of comments in a few functions where they were missing.\n\nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex ba3bef8821..ee86e4de46 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -418,6 +418,7 @@ cmd_set_branch() {\n \tdefault=\n \tbranch=\n \n+\t# parse $args after \"submodule ... set-branch\".\n \twhile test $# -ne 0\n \tdo\n \t\tcase \"$1\" in\n@@ -466,6 +467,7 @@ cmd_set_branch() {\n # $@ = requested path, requested url\n #\n cmd_set_url() {\n+\t# parse $args after \"submodule ... set-url\".\n \twhile test $# -ne 0\n \tdo\n \t\tcase \"$1\" in\n@@ -604,6 +606,7 @@ cmd_status()\n #\n cmd_sync()\n {\n+\t# parse $args after \"submodule ... sync\".\n \twhile test $# -ne 0\n \tdo\n \t\tcase \"$1\" in\n-- \n2.30.2\n\n"},{"id":"508954","messageId":"20241211063234.7610-7-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241211063234.7610-1-royeldar0@gmail.com","subject":"[PATCH v4 6/7] git-submodule.sh: improve variables readability","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-11T06:32:33Z","receivedAt":"2024-12-11T06:33:57Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"When git-submodule.sh parses various options and switches, it sets some\nvariables to values; the variables in turn affect the options given to\ngit-submodule--helper.\n\nCurrently, variables which correspond to switches have boolean values\n(for example, whenever \"--force\" is passed, force=1), while variables\nwhich correspond to options which take arguments have string values that\nsometimes contain the option name and sometimes only the option value.\n\nSet all of the variables to strings which contain the option name (e.g.\nforce=\"--force\" rather than force=1); this has a couple of advantages:\nit improves consistency, readability and debuggability.\n\nSuggested-by: Junio C Hamano \nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 213 +++++++++++++++++++++--------------------------\n 1 file changed, 95 insertions(+), 118 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex ee86e4de46..6df25efc48 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -52,6 +52,10 @@ single_branch=\n jobs=\n recommend_shallow=\n filter=\n+deinit_all=\n+default=\n+summary_limit=\n+for_status=\n \n #\n # Add a new submodule to the working tree, .gitmodules and the index\n@@ -63,37 +67,33 @@ filter=\n cmd_add()\n {\n \t# parse $args after \"submodule ... add\".\n-\treference_path=\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\tbranch=\"--branch=$2\"\n \t\t\tshift\n \t\t\t;;\n-\t\t-b*)\n-\t\t\tbranch=\"${1#-b}\"\n-\t\t\t;;\n-\t\t--branch=*)\n-\t\t\tbranch=\"${1#--branch=}\"\n+\t\t-b* | --branch=*)\n+\t\t\tbranch=\"$1\"\n \t\t\t;;\n \t\t-f | --force)\n \t\t\tforce=$1\n \t\t\t;;\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\t;;\n \t\t--progress)\n-\t\t\tprogress=1\n+\t\t\tprogress=$1\n \t\t\t;;\n \t\t--reference)\n \t\t\tcase \"$2\" in '') usage ;; esac\n-\t\t\treference_path=$2\n+\t\t\treference=\"--reference=$2\"\n \t\t\tshift\n \t\t\t;;\n \t\t--reference=*)\n-\t\t\treference_path=\"${1#--reference=}\"\n+\t\t\treference=\"$1\"\n \t\t\t;;\n \t\t--ref-format)\n \t\t\tcase \"$2\" in '') usage ;; esac\n@@ -104,15 +104,15 @@ cmd_add()\n \t\t\tref_format=\"$1\"\n \t\t\t;;\n \t\t--dissociate)\n-\t\t\tdissociate=1\n+\t\t\tdissociate=$1\n \t\t\t;;\n \t\t--name)\n \t\t\tcase \"$2\" in '') usage ;; esac\n-\t\t\tcustom_name=$2\n+\t\t\tcustom_name=\"--name=$2\"\n \t\t\tshift\n \t\t\t;;\n \t\t--name=*)\n-\t\t\tcustom_name=\"${1#--name=}\"\n+\t\t\tcustom_name=\"$1\"\n \t\t\t;;\n \t\t--depth)\n \t\t\tcase \"$2\" in '') usage ;; esac\n@@ -120,7 +120,7 @@ cmd_add()\n \t\t\tshift\n \t\t\t;;\n \t\t--depth=*)\n-\t\t\tdepth=$1\n+\t\t\tdepth=\"$1\"\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -142,14 +142,14 @@ cmd_add()\n \tfi\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper add \\\n-\t\t${quiet:+--quiet} \\\n-\t\t${force:+--force} \\\n-\t\t${progress:+\"--progress\"} \\\n-\t\t${branch:+--branch \"$branch\"} \\\n-\t\t${reference_path:+--reference \"$reference_path\"} \\\n+\t\t$quiet \\\n+\t\t$force \\\n+\t\t$progress \\\n+\t\t${branch:+\"$branch\"} \\\n+\t\t${reference:+\"$reference\"} \\\n \t\t${ref_format:+\"$ref_format\"} \\\n-\t\t${dissociate:+--dissociate} \\\n-\t\t${custom_name:+--name \"$custom_name\"} \\\n+\t\t$dissociate \\\n+\t\t${custom_name:+\"$custom_name\"} \\\n \t\t${depth:+\"$depth\"} \\\n \t\t-- \\\n \t\t\"$@\"\n@@ -168,10 +168,10 @@ cmd_foreach()\n \tdo\n \t\tcase \"$1\" in\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\t;;\n \t\t--recursive)\n-\t\t\trecursive=1\n+\t\t\trecursive=$1\n \t\t\t;;\n \t\t-*)\n \t\t\tusage\n@@ -184,8 +184,8 @@ cmd_foreach()\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper foreach \\\n-\t\t${quiet:+--quiet} \\\n-\t\t${recursive:+--recursive} \\\n+\t\t$quiet \\\n+\t\t$recursive \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -202,7 +202,7 @@ cmd_init()\n \tdo\n \t\tcase \"$1\" in\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -219,7 +219,7 @@ cmd_init()\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper init \\\n-\t\t${quiet:+--quiet} \\\n+\t\t$quiet \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -230,7 +230,6 @@ cmd_init()\n cmd_deinit()\n {\n \t# parse $args after \"submodule ... deinit\".\n-\tdeinit_all=\n \twhile test $# -ne 0\n \tdo\n \t\tcase \"$1\" in\n@@ -238,10 +237,10 @@ cmd_deinit()\n \t\t\tforce=$1\n \t\t\t;;\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\t;;\n \t\t--all)\n-\t\t\tdeinit_all=t\n+\t\t\tdeinit_all=$1\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -258,9 +257,9 @@ cmd_deinit()\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper deinit \\\n-\t\t${quiet:+--quiet} \\\n-\t\t${force:+--force} \\\n-\t\t${deinit_all:+--all} \\\n+\t\t$quiet \\\n+\t\t$force \\\n+\t\t$deinit_all \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -277,31 +276,31 @@ cmd_update()\n \tdo\n \t\tcase \"$1\" in\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\t;;\n \t\t-v|--verbose)\n-\t\t\tquiet=0\n+\t\t\tquiet=\n \t\t\t;;\n \t\t--progress)\n-\t\t\tprogress=1\n+\t\t\tprogress=$1\n \t\t\t;;\n \t\t-i|--init)\n-\t\t\tinit=1\n+\t\t\tinit=$1\n \t\t\t;;\n \t\t--require-init)\n-\t\t\trequire_init=1\n+\t\t\trequire_init=$1\n \t\t\t;;\n \t\t--remote)\n-\t\t\tremote=1\n+\t\t\tremote=$1\n \t\t\t;;\n \t\t-N|--no-fetch)\n-\t\t\tnofetch=1\n+\t\t\tnofetch=$1\n \t\t\t;;\n \t\t-f|--force)\n \t\t\tforce=$1\n \t\t\t;;\n \t\t-r|--rebase)\n-\t\t\trebase=1\n+\t\t\trebase=$1\n \t\t\t;;\n \t\t--ref-format)\n \t\t\tcase \"$2\" in '') usage ;; esac\n@@ -320,22 +319,19 @@ cmd_update()\n \t\t\treference=\"$1\"\n \t\t\t;;\n \t\t--dissociate)\n-\t\t\tdissociate=1\n+\t\t\tdissociate=$1\n \t\t\t;;\n \t\t-m|--merge)\n-\t\t\tmerge=1\n+\t\t\tmerge=$1\n \t\t\t;;\n \t\t--recursive)\n-\t\t\trecursive=1\n+\t\t\trecursive=$1\n \t\t\t;;\n \t\t--checkout)\n-\t\t\tcheckout=1\n-\t\t\t;;\n-\t\t--recommend-shallow)\n-\t\t\trecommend_shallow=\"--recommend-shallow\"\n+\t\t\tcheckout=$1\n \t\t\t;;\n-\t\t--no-recommend-shallow)\n-\t\t\trecommend_shallow=\"--no-recommend-shallow\"\n+\t\t--recommend-shallow|--no-recommend-shallow)\n+\t\t\trecommend_shallow=$1\n \t\t\t;;\n \t\t--depth)\n \t\t\tcase \"$2\" in '') usage ;; esac\n@@ -343,24 +339,18 @@ cmd_update()\n \t\t\tshift\n \t\t\t;;\n \t\t--depth=*)\n-\t\t\tdepth=$1\n+\t\t\tdepth=\"$1\"\n \t\t\t;;\n \t\t-j|--jobs)\n \t\t\tcase \"$2\" in '') usage ;; esac\n \t\t\tjobs=\"--jobs=$2\"\n \t\t\tshift\n \t\t\t;;\n-\t\t-j*)\n-\t\t\tjobs=\"--jobs=${1#-j}\"\n-\t\t\t;;\n-\t\t--jobs=*)\n-\t\t\tjobs=$1\n+\t\t-j*|--jobs=*)\n+\t\t\tjobs=\"$1\"\n \t\t\t;;\n-\t\t--single-branch)\n-\t\t\tsingle_branch=\"--single-branch\"\n-\t\t\t;;\n-\t\t--no-single-branch)\n-\t\t\tsingle_branch=\"--no-single-branch\"\n+\t\t--single-branch|--no-single-branch)\n+\t\t\tsingle_branch=$1\n \t\t\t;;\n \t\t--filter)\n \t\t\tcase \"$2\" in '') usage ;; esac\n@@ -385,22 +375,21 @@ cmd_update()\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper update \\\n-\t\t${quiet:+--quiet} \\\n-\t\t${force:+--force} \\\n-\t\t${progress:+\"--progress\"} \\\n-\t\t${remote:+--remote} \\\n-\t\t${recursive:+--recursive} \\\n-\t\t${init:+--init} \\\n-\t\t${nofetch:+--no-fetch} \\\n-\t\t${rebase:+--rebase} \\\n-\t\t${merge:+--merge} \\\n-\t\t${checkout:+--checkout} \\\n+\t\t$quiet \\\n+\t\t$force \\\n+\t\t$progress \\\n+\t\t$remote \\\n+\t\t$recursive \\\n+\t\t$init \\\n+\t\t$nofetch \\\n+\t\t$rebase \\\n+\t\t$merge \\\n+\t\t$checkout \\\n \t\t${ref_format:+\"$ref_format\"} \\\n \t\t${reference:+\"$reference\"} \\\n-\t\t${dissociate:+\"--dissociate\"} \\\n+\t\t$dissociate \\\n \t\t${depth:+\"$depth\"} \\\n-\t\t${require_init:+--require-init} \\\n-\t\t${dissociate:+\"--dissociate\"} \\\n+\t\t$require_init \\\n \t\t$single_branch \\\n \t\t$recommend_shallow \\\n \t\t$jobs \\\n@@ -415,9 +404,6 @@ cmd_update()\n # $@ = requested path\n #\n cmd_set_branch() {\n-\tdefault=\n-\tbranch=\n-\n \t# parse $args after \"submodule ... set-branch\".\n \twhile test $# -ne 0\n \tdo\n@@ -426,18 +412,15 @@ cmd_set_branch() {\n \t\t\t# we don't do anything with this but we need to accept it\n \t\t\t;;\n \t\t-d|--default)\n-\t\t\tdefault=1\n+\t\t\tdefault=$1\n \t\t\t;;\n \t\t-b|--branch)\n \t\t\tcase \"$2\" in '') usage ;; esac\n-\t\t\tbranch=$2\n+\t\t\tbranch=\"--branch=$2\"\n \t\t\tshift\n \t\t\t;;\n-\t\t-b*)\n-\t\t\tbranch=\"${1#-b}\"\n-\t\t\t;;\n-\t\t--branch=*)\n-\t\t\tbranch=\"${1#--branch=}\"\n+\t\t-b*|--branch=*)\n+\t\t\tbranch=\"$1\"\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -454,9 +437,9 @@ cmd_set_branch() {\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper set-branch \\\n-\t\t${quiet:+--quiet} \\\n-\t\t${branch:+--branch \"$branch\"} \\\n-\t\t${default:+--default} \\\n+\t\t$quiet \\\n+\t\t${branch:+\"$branch\"} \\\n+\t\t$default \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -472,7 +455,7 @@ cmd_set_url() {\n \tdo\n \t\tcase \"$1\" in\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -489,7 +472,7 @@ cmd_set_url() {\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper set-url \\\n-\t\t${quiet:+--quiet} \\\n+\t\t$quiet \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -503,32 +486,26 @@ cmd_set_url() {\n # $@ = [commit (default 'HEAD'),] requested paths (default all)\n #\n cmd_summary() {\n-\tsummary_limit=-1\n-\tfor_status=\n-\n \t# parse $args after \"submodule ... summary\".\n \twhile test $# -ne 0\n \tdo\n \t\tcase \"$1\" in\n \t\t--cached)\n-\t\t\tcached=1\n+\t\t\tcached=$1\n \t\t\t;;\n \t\t--files)\n-\t\t\tfiles=\"$1\"\n+\t\t\tfiles=$1\n \t\t\t;;\n \t\t--for-status)\n-\t\t\tfor_status=\"$1\"\n+\t\t\tfor_status=$1\n \t\t\t;;\n \t\t-n|--summary-limit)\n \t\t\tcase \"$2\" in '') usage ;; esac\n-\t\t\tsummary_limit=\"$2\"\n+\t\t\tsummary_limit=\"--summary-limit=$2\"\n \t\t\tshift\n \t\t\t;;\n-\t\t-n*)\n-\t\t\tsummary_limit=\"${1#-n}\"\n-\t\t\t;;\n-\t\t--summary-limit=*)\n-\t\t\tsummary_limit=\"${1#--summary-limit=}\"\n+\t\t-n*|--summary-limit=*)\n+\t\t\tsummary_limit=\"$1\"\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -545,10 +522,10 @@ cmd_summary() {\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper summary \\\n-\t\t${files:+--files} \\\n-\t\t${cached:+--cached} \\\n-\t\t${for_status:+--for-status} \\\n-\t\t${summary_limit:+-n \"$summary_limit\"} \\\n+\t\t$files \\\n+\t\t$cached \\\n+\t\t$for_status \\\n+\t\t${summary_limit:+\"$summary_limit\"} \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -569,13 +546,13 @@ cmd_status()\n \tdo\n \t\tcase \"$1\" in\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\t;;\n \t\t--cached)\n-\t\t\tcached=1\n+\t\t\tcached=$1\n \t\t\t;;\n \t\t--recursive)\n-\t\t\trecursive=1\n+\t\t\trecursive=$1\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -592,9 +569,9 @@ cmd_status()\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper status \\\n-\t\t${quiet:+--quiet} \\\n-\t\t${cached:+--cached} \\\n-\t\t${recursive:+--recursive} \\\n+\t\t$quiet \\\n+\t\t$cached \\\n+\t\t$recursive \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -611,11 +588,11 @@ cmd_sync()\n \tdo\n \t\tcase \"$1\" in\n \t\t-q|--quiet)\n-\t\t\tquiet=1\n+\t\t\tquiet=$1\n \t\t\tshift\n \t\t\t;;\n \t\t--recursive)\n-\t\t\trecursive=1\n+\t\t\trecursive=$1\n \t\t\tshift\n \t\t\t;;\n \t\t--)\n@@ -632,8 +609,8 @@ cmd_sync()\n \tdone\n \n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper sync \\\n-\t\t${quiet:+--quiet} \\\n-\t\t${recursive:+--recursive} \\\n+\t\t$quiet \\\n+\t\t$recursive \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -656,10 +633,10 @@ do\n \t\tcommand=$1\n \t\t;;\n \t-q|--quiet)\n-\t\tquiet=1\n+\t\tquiet=$1\n \t\t;;\n \t--cached)\n-\t\tcached=1\n+\t\tcached=$1\n \t\t;;\n \t--)\n \t\tbreak\n-- \n2.30.2\n\n"},{"id":"508955","messageId":"20241211063234.7610-8-royeldar0@gmail.com","threadId":"62611","inReplyTo":"20241211063234.7610-1-royeldar0@gmail.com","subject":"[PATCH v4 7/7] git-submodule.sh: rename some variables","fromName":"Roy Eldar","fromEmail":"royeldar0@gmail.com","sentAt":"2024-12-11T06:32:34Z","receivedAt":"2024-12-11T06:34:01Z","isPatch":true,"sender":{"key":"royeldar0@gmail.com","avatar":null},"body":"Every switch and option which is passed to git-submodule.sh has a\ncorresponding variable which is set accordingly; by convention, the name\nof the variable is the option name (for example, \"--jobs\" and \"$jobs\").\n\nRename \"$custom_name\", \"$deinit_all\" and \"$nofetch\", for consistency.\n\nSigned-off-by: Roy Eldar \n---\n git-submodule.sh | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 6df25efc48..2999b31fad 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -40,11 +40,11 @@ init=\n require_init=\n files=\n remote=\n-nofetch=\n+no_fetch=\n rebase=\n merge=\n checkout=\n-custom_name=\n+name=\n depth=\n progress=\n dissociate=\n@@ -52,7 +52,7 @@ single_branch=\n jobs=\n recommend_shallow=\n filter=\n-deinit_all=\n+all=\n default=\n summary_limit=\n for_status=\n@@ -108,11 +108,11 @@ cmd_add()\n \t\t\t;;\n \t\t--name)\n \t\t\tcase \"$2\" in '') usage ;; esac\n-\t\t\tcustom_name=\"--name=$2\"\n+\t\t\tname=\"--name=$2\"\n \t\t\tshift\n \t\t\t;;\n \t\t--name=*)\n-\t\t\tcustom_name=\"$1\"\n+\t\t\tname=\"$1\"\n \t\t\t;;\n \t\t--depth)\n \t\t\tcase \"$2\" in '') usage ;; esac\n@@ -149,7 +149,7 @@ cmd_add()\n \t\t${reference:+\"$reference\"} \\\n \t\t${ref_format:+\"$ref_format\"} \\\n \t\t$dissociate \\\n-\t\t${custom_name:+\"$custom_name\"} \\\n+\t\t${name:+\"$name\"} \\\n \t\t${depth:+\"$depth\"} \\\n \t\t-- \\\n \t\t\"$@\"\n@@ -240,7 +240,7 @@ cmd_deinit()\n \t\t\tquiet=$1\n \t\t\t;;\n \t\t--all)\n-\t\t\tdeinit_all=$1\n+\t\t\tall=$1\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -259,7 +259,7 @@ cmd_deinit()\n \tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper deinit \\\n \t\t$quiet \\\n \t\t$force \\\n-\t\t$deinit_all \\\n+\t\t$all \\\n \t\t-- \\\n \t\t\"$@\"\n }\n@@ -294,7 +294,7 @@ cmd_update()\n \t\t\tremote=$1\n \t\t\t;;\n \t\t-N|--no-fetch)\n-\t\t\tnofetch=$1\n+\t\t\tno_fetch=$1\n \t\t\t;;\n \t\t-f|--force)\n \t\t\tforce=$1\n@@ -381,7 +381,7 @@ cmd_update()\n \t\t$remote \\\n \t\t$recursive \\\n \t\t$init \\\n-\t\t$nofetch \\\n+\t\t$no_fetch \\\n \t\t$rebase \\\n \t\t$merge \\\n \t\t$checkout \\\n-- \n2.30.2\n\n"}]}