{"thread":{"id":"28622","subject":"[PATCH 1/2] submodule: whitespace fix","startedAt":"2011-10-07T09:04:54Z","lastAt":"2011-10-21T13:49:36Z","messageCount":9,"participants":["Tay Ray Chuan","Jens Lehmann"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"177144","messageId":"1317978295-4796-1-git-send-email-rctay89@gmail.com","threadId":"28622","inReplyTo":null,"subject":"[PATCH 1/2] submodule: whitespace fix","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2011-10-07T09:04:54Z","receivedAt":"2011-10-07T09:04:54Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Replace SPs with TAB.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n git-submodule.sh |    6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 928a62f..ebea35b 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -104,9 +104,9 @@ module_name()\n \tre=$(printf '%s\\n' \"$1\" | sed -e 's/[].[^$\\\\*]/\\\\&/g')\n \tname=$( git config -f .gitmodules --get-regexp '^submodule\\..*\\.path$' |\n \t\tsed -n -e 's|^submodule\\.\\(.*\\)\\.path '\"$re\"'$|\\1|p' )\n-       test -z \"$name\" &&\n-       die \"$(eval_gettext \"No submodule mapping found in .gitmodules for path '\\$path'\")\"\n-       echo \"$name\"\n+\ttest -z \"$name\" &&\n+\tdie \"$(eval_gettext \"No submodule mapping found in .gitmodules for path '\\$path'\")\"\n+\techo \"$name\"\n }\n \n #\n-- \n1.7.7.584.g16d0ea\n"},{"id":"177145","messageId":"1317978295-4796-2-git-send-email-rctay89@gmail.com","threadId":"28622","inReplyTo":"1317978295-4796-1-git-send-email-rctay89@gmail.com","subject":"[PATCH 2/2] submodule::module_clone(): silence die() message from module_name()","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2011-10-07T09:04:55Z","receivedAt":"2011-10-07T09:04:55Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"The die() message that may occur in module_name() is not really relevant\nto the user when called from module_clone(); the latter handles the\n\"failure\" (no submodule mapping) anyway.\n\nLeave other callers of module_name() unchanged, as the die() message\nshown is either relevant for user consumption (such as those that exit()\nwhen the call fails), or will not occur at all (when called with paths\nreturned by module_list()).\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n git-submodule.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex ebea35b..3adab93 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -130,7 +130,7 @@ module_clone()\n \n \tgitdir=\n \tgitdir_base=\n-\tname=$(module_name \"$path\")\n+\tname=$(module_name \"$path\" 2>/dev/null)\n \tbase_path=$(dirname \"$path\")\n \n \tgitdir=$(git rev-parse --git-dir)\n-- \n1.7.7.584.g16d0ea\n"},{"id":"177323","messageId":"4E933EDF.6020909@web.de","threadId":"28622","inReplyTo":"1317978295-4796-1-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH 1/2] submodule: whitespace fix","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-10-10T18:52:15Z","receivedAt":"2011-10-10T18:52:15Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 07.10.2011 11:04, schrieb Tay Ray Chuan:\n> Replace SPs with TAB.\n\nAs these are the only lines where spaces are used for indentation in this\nfile this might be a worthwhile cleanup, but I really don't care that much.\nJunio, what do you think?\n\n> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>\n> ---\n>  git-submodule.sh |    6 +++---\n>  1 files changed, 3 insertions(+), 3 deletions(-)\n> \n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 928a62f..ebea35b 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -104,9 +104,9 @@ module_name()\n>  \tre=$(printf '%s\\n' \"$1\" | sed -e 's/[].[^$\\\\*]/\\\\&/g')\n>  \tname=$( git config -f .gitmodules --get-regexp '^submodule\\..*\\.path$' |\n>  \t\tsed -n -e 's|^submodule\\.\\(.*\\)\\.path '\"$re\"'$|\\1|p' )\n> -       test -z \"$name\" &&\n> -       die \"$(eval_gettext \"No submodule mapping found in .gitmodules for path '\\$path'\")\"\n> -       echo \"$name\"\n> +\ttest -z \"$name\" &&\n> +\tdie \"$(eval_gettext \"No submodule mapping found in .gitmodules for path '\\$path'\")\"\n> +\techo \"$name\"\n>  }\n>  \n>  #\n"},{"id":"177324","messageId":"4E9348A8.5000500@web.de","threadId":"28622","inReplyTo":"1317978295-4796-2-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH 2/2] submodule::module_clone(): silence die() message from module_name()","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-10-10T19:34:00Z","receivedAt":"2011-10-10T19:34:00Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"BTW: this patch applies to next\n\nAm 07.10.2011 11:04, schrieb Tay Ray Chuan:\n> The die() message that may occur in module_name() is not really relevant\n> to the user when called from module_clone(); the latter handles the\n> \"failure\" (no submodule mapping) anyway.\n\nMakes tons of sense, especially as adding a new submodule currently always\nspews out the \"No submodule mapping found in .gitmodules for path 'sub'\"\nmessage right before that mapping is added there. Thanks for noticing that\nand ACK on that change from my side.\n\n> Leave other callers of module_name() unchanged, as the die() message\n> shown is either relevant for user consumption (such as those that exit()\n> when the call fails), or will not occur at all (when called with paths\n> returned by module_list()).\n\nHmm, while I agree on the first reasoning I'm not sure about the second.\nmodule_list() asks the index for the submodule paths while module_name()\ngets it's input from .gitmodules, so they can (and sometimes will)\ndisagree. When cmd_foreach() passes an empty \"name\" variable to the\nspawned command that might still work (and even make sense), but using the\nempty name in cmd_sync() to access the config is looking like an error to\nme. It might make sense to add an \"|| exit\" at least to the callsite in\ncmd_sync(). Or am I missing something here?\n\n> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>\n> ---\n>  git-submodule.sh |    2 +-\n>  1 files changed, 1 insertions(+), 1 deletions(-)\n> \n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index ebea35b..3adab93 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -130,7 +130,7 @@ module_clone()\n>  \n>  \tgitdir=\n>  \tgitdir_base=\n> -\tname=$(module_name \"$path\")\n> +\tname=$(module_name \"$path\" 2>/dev/null)\n>  \tbase_path=$(dirname \"$path\")\n>  \n>  \tgitdir=$(git rev-parse --git-dir)\n"},{"id":"177360","messageId":"CALUzUxo6YEEpL_MhT=O9sJSUdwcpKBpeM2O8GkbbyxvqmWCFLQ@mail.gmail.com","threadId":"28622","inReplyTo":"4E9348A8.5000500@web.de","subject":"Re: [PATCH 2/2] submodule::module_clone(): silence die() message from module_name()","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2011-10-11T08:44:34Z","receivedAt":"2011-10-11T08:44:34Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"On Tue, Oct 11, 2011 at 3:34 AM, Jens Lehmann <Jens.Lehmann@web.de> wrote:\n> BTW: this patch applies to next\n>\n> Am 07.10.2011 11:04, schrieb Tay Ray Chuan:\n>> The die() message that may occur in module_name() is not really relevant\n>> to the user when called from module_clone(); the latter handles the\n>> \"failure\" (no submodule mapping) anyway.\n>\n> Makes tons of sense, especially as adding a new submodule currently always\n> spews out the \"No submodule mapping found in .gitmodules for path 'sub'\"\n> message right before that mapping is added there. Thanks for noticing that\n> and ACK on that change from my side.\n\nThanks for the review.\n\n>> Leave other callers of module_name() unchanged, as the die() message\n>> shown is either relevant for user consumption (such as those that exit()\n>> when the call fails), or will not occur at all (when called with paths\n>> returned by module_list()).\n>\n> Hmm, while I agree on the first reasoning I'm not sure about the second.\n> module_list() asks the index for the submodule paths while module_name()\n> gets it's input from .gitmodules, so they can (and sometimes will)\n> disagree.\n\nOh, you're right. I neglected to see how module_list() actually worked.\n\n> When cmd_foreach() passes an empty \"name\" variable to the\n> spawned command that might still work (and even make sense), but using the\n> empty name in cmd_sync() to access the config is looking like an error to\n> me. It might make sense to add an \"|| exit\" at least to the callsite in\n> cmd_sync(). Or am I missing something here?\n\nCc-ed David, who authored cmd_sync().\n\nDavid, what do you think of Jens' analysis?\n\nIn the meantime, I'll probably reword the second paragraph to say that\nfuture work will be needed to analyze non- || exit callsites.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"177382","messageId":"4E947F01.8040600@web.de","threadId":"28622","inReplyTo":"CALUzUxo6YEEpL_MhT=O9sJSUdwcpKBpeM2O8GkbbyxvqmWCFLQ@mail.gmail.com","subject":"Re: [PATCH 2/2] submodule::module_clone(): silence die() message from module_name()","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-10-11T17:38:09Z","receivedAt":"2011-10-11T17:38:09Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 11.10.2011 10:44, schrieb Tay Ray Chuan:\n> On Tue, Oct 11, 2011 at 3:34 AM, Jens Lehmann <Jens.Lehmann@web.de> wrote:\n>> When cmd_foreach() passes an empty \"name\" variable to the\n>> spawned command that might still work (and even make sense), but using the\n>> empty name in cmd_sync() to access the config is looking like an error to\n>> me. It might make sense to add an \"|| exit\" at least to the callsite in\n>> cmd_sync(). Or am I missing something here?\n> \n> Cc-ed David, who authored cmd_sync().\n> \n> David, what do you think of Jens' analysis?\n> \n> In the meantime, I'll probably reword the second paragraph to say that\n> future work will be needed to analyze non- || exit callsites.\n\nYeah, me too thinks the missing \"|| exit\" should be subject of another\npatch.\n"},{"id":"178125","messageId":"1319204976-5076-1-git-send-email-rctay89@gmail.com","threadId":"28622","inReplyTo":"1317978295-4796-2-git-send-email-rctay89@gmail.com","subject":"[PATCH v2 0/2] submodule patches","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2011-10-21T13:49:34Z","receivedAt":"2011-10-21T13:49:34Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Junio, this goes on top of 'fg/submodule-git-file-git-dir' (particularly\nthe second patch).\n\nChanged in v2: reworded 2nd paragraph of 2nd patch, as recommended by\nJens.\n\nTay Ray Chuan (2):\n  submodule: whitespace fix\n  submodule::module_clone(): silence die() message from module_name()\n\n git-submodule.sh |    8 ++++----\n 1 files changed, 4 insertions(+), 4 deletions(-)\n\n-- \n1.7.6.msysgit.0.584.g2cbf\n"},{"id":"178126","messageId":"1319204976-5076-2-git-send-email-rctay89@gmail.com","threadId":"28622","inReplyTo":"1319204976-5076-1-git-send-email-rctay89@gmail.com","subject":"[PATCH v2 1/2] submodule: whitespace fix","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2011-10-21T13:49:35Z","receivedAt":"2011-10-21T13:49:35Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Replace SPs with TAB.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n git-submodule.sh |    6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 7576d14..8e9e5ea 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -104,9 +104,9 @@ module_name()\n \tre=$(printf '%s\\n' \"$1\" | sed -e 's/[].[^$\\\\*]/\\\\&/g')\n \tname=$( git config -f .gitmodules --get-regexp '^submodule\\..*\\.path$' |\n \t\tsed -n -e 's|^submodule\\.\\(.*\\)\\.path '\"$re\"'$|\\1|p' )\n-       test -z \"$name\" &&\n-       die \"$(eval_gettext \"No submodule mapping found in .gitmodules for path '\\$path'\")\"\n-       echo \"$name\"\n+\ttest -z \"$name\" &&\n+\tdie \"$(eval_gettext \"No submodule mapping found in .gitmodules for path '\\$path'\")\"\n+\techo \"$name\"\n }\n \n #\n-- \n1.7.6.msysgit.0.584.g2cbf\n"},{"id":"178127","messageId":"1319204976-5076-3-git-send-email-rctay89@gmail.com","threadId":"28622","inReplyTo":"1319204976-5076-2-git-send-email-rctay89@gmail.com","subject":"[PATCH v2 2/2] submodule::module_clone(): silence die() message from module_name()","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2011-10-21T13:49:36Z","receivedAt":"2011-10-21T13:49:36Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"The die() message that may occur in module_name() is not really relevant\nto the user when called from module_clone(); the latter handles the\n\"failure\" (no submodule mapping) anyway.\n\nAnalysis of other callsites is left to future work.\n\nAcked-by: Jens Lehmann <Jens.Lehmann@web.de>\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n git-submodule.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 8e9e5ea..5d29f82 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -124,7 +124,7 @@ module_clone()\n \treference=\"$3\"\n \tgitdir=\n \tgitdir_base=\n-\tname=$(module_name \"$path\")\n+\tname=$(module_name \"$path\" 2>/dev/null)\n \tbase_path=$(dirname \"$path\")\n \n \tgitdir=$(git rev-parse --git-dir)\n-- \n1.7.6.msysgit.0.584.g2cbf\n"}]}