{"thread":{"id":"31420","subject":"[PATCH] git-submodule: respect -q for add/update","startedAt":"2012-09-04T07:31:20Z","lastAt":"2012-09-05T20:33:39Z","messageCount":4,"participants":["Orgad Shaneh","Jens Lehmann","Orgad and Raizel Shaneh"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"198278","messageId":"1346743880-11379-1-git-send-email-orgads@gmail.com","threadId":"31420","inReplyTo":null,"subject":"[PATCH] git-submodule: respect -q for add/update","fromName":"Orgad Shaneh","fromEmail":"orgads@gmail.com","sentAt":"2012-09-04T07:31:20Z","receivedAt":"2012-09-04T07:31:20Z","isPatch":true,"sender":{"key":"orgads@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1246544?v=4"},"body":"Signed-off-by: Orgad Shaneh <orgads@gmail.com>\n---\n git-submodule.sh |   17 ++++++++++++++---\n 1 file changed, 14 insertions(+), 3 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex aac575e..dd57abb 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -266,6 +266,11 @@ cmd_add()\n \n \trepo=$1\n \tsm_path=$2\n+\tquiet=\n+\tif test -n \"$GIT_QUIET\"\n+\tthen\n+\t\tquiet=-q\n+\tfi\n \n \tif test -z \"$sm_path\"; then\n \t\tsm_path=$(echo \"$repo\" |\n@@ -332,8 +337,8 @@ Use -f if you really want to add it.\" >&2\n \t\t\tcd \"$sm_path\" &&\n \t\t\t# ash fails to wordsplit ${branch:+-b \"$branch\"...}\n \t\t\tcase \"$branch\" in\n-\t\t\t'') git checkout -f -q ;;\n-\t\t\t?*) git checkout -f -q -B \"$branch\" \"origin/$branch\" ;;\n+\t\t\t'') git checkout -f $quiet ;;\n+\t\t\t?*) git checkout -f $quiet -B \"$branch\" \"origin/$branch\" ;;\n \t\t\tesac\n \t\t) || die \"$(eval_gettext \"Unable to checkout submodule '\\$sm_path'\")\"\n \tfi\n@@ -527,6 +532,12 @@ cmd_update()\n \t\tshift\n \tdone\n \n+\tquiet=\n+\tif test -n \"$GIT_QUIET\"\n+\tthen\n+\t\tquiet=-q\n+\tfi\n+\n \tif test -n \"$init\"\n \tthen\n \t\tcmd_init \"--\" \"$@\" || return\n@@ -619,7 +630,7 @@ Maybe you want to use 'update --init'?\")\"\n \t\t\t\tmust_die_on_failure=yes\n \t\t\t\t;;\n \t\t\t*)\n-\t\t\t\tcommand=\"git checkout $subforce -q\"\n+\t\t\t\tcommand=\"git checkout $subforce $quiet\"\n \t\t\t\tdie_msg=\"$(eval_gettext \"Unable to checkout '\\$sha1' in submodule path '\\$sm_path'\")\"\n \t\t\t\tsay_msg=\"$(eval_gettext \"Submodule path '\\$sm_path': checked out '\\$sha1'\")\"\n \t\t\t\t;;\n-- \n1.7.10.4\n"},{"id":"198305","messageId":"50461E18.6020400@web.de","threadId":"31420","inReplyTo":"1346743880-11379-1-git-send-email-orgads@gmail.com","subject":"Re: [PATCH] git-submodule: respect -q for add/update","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-09-04T15:28:24Z","receivedAt":"2012-09-04T15:28:24Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 04.09.2012 09:31, schrieb Orgad Shaneh:\n> Signed-off-by: Orgad Shaneh <orgads@gmail.com>\n\nBefore the \"Signed-off-by\" is the place where you should have\nexplained why this would be a worthwhile change ;-)\n\nTo me this looks like you make the default noisier and require an\nexplicit \"-q\" to make it quiet again. There is a reason you don't\nnormally get bothered with the output of the checkout command run\nunder the hood of git submodule add/update, so I don't think this\nchange makes things better.\n\nBut you might want to think about adding a \"-v/--verbose\" flag to\nmake the submodule add/update checkouts more verbose, in case you\ncare about the output of the checkout command. That would be a\nsane thing to do, so what about changing your patch into this\ndirection?\n\n> ---\n>  git-submodule.sh |   17 ++++++++++++++---\n>  1 file changed, 14 insertions(+), 3 deletions(-)\n> \n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index aac575e..dd57abb 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -266,6 +266,11 @@ cmd_add()\n>  \n>  \trepo=$1\n>  \tsm_path=$2\n> +\tquiet=\n> +\tif test -n \"$GIT_QUIET\"\n> +\tthen\n> +\t\tquiet=-q\n> +\tfi\n>  \n>  \tif test -z \"$sm_path\"; then\n>  \t\tsm_path=$(echo \"$repo\" |\n> @@ -332,8 +337,8 @@ Use -f if you really want to add it.\" >&2\n>  \t\t\tcd \"$sm_path\" &&\n>  \t\t\t# ash fails to wordsplit ${branch:+-b \"$branch\"...}\n>  \t\t\tcase \"$branch\" in\n> -\t\t\t'') git checkout -f -q ;;\n> -\t\t\t?*) git checkout -f -q -B \"$branch\" \"origin/$branch\" ;;\n> +\t\t\t'') git checkout -f $quiet ;;\n> +\t\t\t?*) git checkout -f $quiet -B \"$branch\" \"origin/$branch\" ;;\n>  \t\t\tesac\n>  \t\t) || die \"$(eval_gettext \"Unable to checkout submodule '\\$sm_path'\")\"\n>  \tfi\n> @@ -527,6 +532,12 @@ cmd_update()\n>  \t\tshift\n>  \tdone\n>  \n> +\tquiet=\n> +\tif test -n \"$GIT_QUIET\"\n> +\tthen\n> +\t\tquiet=-q\n> +\tfi\n> +\n>  \tif test -n \"$init\"\n>  \tthen\n>  \t\tcmd_init \"--\" \"$@\" || return\n> @@ -619,7 +630,7 @@ Maybe you want to use 'update --init'?\")\"\n>  \t\t\t\tmust_die_on_failure=yes\n>  \t\t\t\t;;\n>  \t\t\t*)\n> -\t\t\t\tcommand=\"git checkout $subforce -q\"\n> +\t\t\t\tcommand=\"git checkout $subforce $quiet\"\n>  \t\t\t\tdie_msg=\"$(eval_gettext \"Unable to checkout '\\$sha1' in submodule path '\\$sm_path'\")\"\n>  \t\t\t\tsay_msg=\"$(eval_gettext \"Submodule path '\\$sm_path': checked out '\\$sha1'\")\"\n>  \t\t\t\t;;\n> \n"},{"id":"198374","messageId":"CAGHpTBJ6hWJKaQvN4oByrest_FB1fGDKa=ge4fM1HWmSWZ1tkw@mail.gmail.com","threadId":"31420","inReplyTo":"50461E18.6020400@web.de","subject":"Re: [PATCH] git-submodule: respect -q for add/update","fromName":"Orgad and Raizel Shaneh","fromEmail":"orgads@gmail.com","sentAt":"2012-09-05T11:42:31Z","receivedAt":"2012-09-05T11:42:31Z","isPatch":true,"sender":{"key":"orgads@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1246544?v=4"},"body":"On Tue, Sep 4, 2012 at 6:28 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:\n>\n> Am 04.09.2012 09:31, schrieb Orgad Shaneh:\n> > Signed-off-by: Orgad Shaneh <orgads@gmail.com>\n>\n> Before the \"Signed-off-by\" is the place where you should have\n> explained why this would be a worthwhile change ;-)\n>\n> To me this looks like you make the default noisier and require an\n> explicit \"-q\" to make it quiet again. There is a reason you don't\n> normally get bothered with the output of the checkout command run\n> under the hood of git submodule add/update, so I don't think this\n> change makes things better.\n>\n> But you might want to think about adding a \"-v/--verbose\" flag to\n> make the submodule add/update checkouts more verbose, in case you\n> care about the output of the checkout command. That would be a\n> sane thing to do, so what about changing your patch into this\n> direction?\n>\n\nI don't agree the default should be quiet. That's what the (submodule)\n-q flag is there for.\n\nWhen I run 'git submodule update' I don't expect to be in the dark\nuntil the submodule/s finishes checkout, this sometimes can take a\nsignificant amount of time and feedback is expected.\n\n- Orgad\n"},{"id":"198402","messageId":"5047B723.4050201@web.de","threadId":"31420","inReplyTo":"CAGHpTBJ6hWJKaQvN4oByrest_FB1fGDKa=ge4fM1HWmSWZ1tkw@mail.gmail.com","subject":"Re: [PATCH] git-submodule: respect -q for add/update","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-09-05T20:33:39Z","receivedAt":"2012-09-05T20:33:39Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 05.09.2012 13:42, schrieb Orgad and Raizel Shaneh:\n> On Tue, Sep 4, 2012 at 6:28 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:\n>>\n>> Am 04.09.2012 09:31, schrieb Orgad Shaneh:\n>>> Signed-off-by: Orgad Shaneh <orgads@gmail.com>\n>>\n>> Before the \"Signed-off-by\" is the place where you should have\n>> explained why this would be a worthwhile change ;-)\n>>\n>> To me this looks like you make the default noisier and require an\n>> explicit \"-q\" to make it quiet again. There is a reason you don't\n>> normally get bothered with the output of the checkout command run\n>> under the hood of git submodule add/update, so I don't think this\n>> change makes things better.\n>>\n>> But you might want to think about adding a \"-v/--verbose\" flag to\n>> make the submodule add/update checkouts more verbose, in case you\n>> care about the output of the checkout command. That would be a\n>> sane thing to do, so what about changing your patch into this\n>> direction?\n>>\n> \n> I don't agree the default should be quiet. That's what the (submodule)\n> -q flag is there for.\n\nNope, the -q flag is to silence *all* output except errors. And it\nmakes perfect sense for high level commands to hide the output of\nthe commands run under the hood like we do here.\n\n> When I run 'git submodule update' I don't expect to be in the dark\n> until the submodule/s finishes checkout, this sometimes can take a\n> significant amount of time and feedback is expected.\n\nAs I said, add a verbose flag so you can see in detail what is going\non.\n"}]}