{"thread":{"id":"20669","subject":"[PATCH next] t7407: use 'cut' utility rather than bash's substring expansion notation","startedAt":"2009-08-20T01:24:52Z","lastAt":"2009-08-20T16:59:57Z","messageCount":5,"participants":["Brandon Casey","Johan Herland","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"121305","messageId":"QHfHFS_5JGiL-O8GMDfdfscFUdxV1xVObzr6e5LPleagDRd7bCg8I9YUwL9xkbgM64vyf_EVLLg@cipher.nrlssc.navy.mil","threadId":"20669","inReplyTo":null,"subject":"[PATCH next] t7407: use 'cut' utility rather than bash's substring expansion notation","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2009-08-20T01:24:52Z","receivedAt":"2009-08-20T01:24:52Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey \n\nThe substring expansion notation is a bashism that we have not so far\nadopted. There is precedence for using the 'cut' utility for extracting\na substring. So do so here.\n\nSigned-off-by: Brandon Casey \n---\n t/t7407-submodule-foreach.sh | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh\nindex de1730d..44ea8ac 100755\n--- a/t/t7407-submodule-foreach.sh\n+++ b/t/t7407-submodule-foreach.sh\n@@ -207,8 +207,8 @@ cat > expect < From: Brandon Casey \n>\n> The substring expansion notation is a bashism that we have not so far\n> adopted. There is precedence for using the 'cut' utility for extracting\n> a substring. So do so here.\n>\n> Signed-off-by: Brandon Casey \n> ---\n> t/t7407-submodule-foreach.sh | 4 ++--\n> 1 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh\n> index de1730d..44ea8ac 100755\n> --- a/t/t7407-submodule-foreach.sh\n> +++ b/t/t7407-submodule-foreach.sh\n> @@ -207,8 +207,8 @@ cat > expect < $nested2sha1 nested1/nested2 (heads/master)\n> $nested3sha1 nested1/nested2/nested3 (heads/master)\n> $submodulesha1 nested1/nested2/nested3/submodule (heads/master)\n> - $sub1sha1 sub1 (${sub1sha1:0:7})\n> - $sub2sha1 sub2 (${sub1sha1:0:7})\n> + $sub1sha1 sub1 ($(echo $sub1sha1 | cut -c 1-7))\n> + $sub2sha1 sub2 ($(echo $sub1sha1 | cut -c 1-7))\n\nTypo (both in the original, and the patch), should be:\n\t$sub2sha1 sub2 ($(echo $sub2sha1 | cut -c 1-7))\n\n> $sub3sha1 sub3 (heads/master)\n> EOF\n\nOtherwise:\n\nAcked-by: Johan Herland \n\nThanks,\n\n\n...Johan\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"121317","messageId":"7vskfnq6q5.fsf@alter.siamese.dyndns.org","threadId":"20669","inReplyTo":"200908200856.30359.johan@herland.net","subject":"Re: [PATCH next] t7407: use 'cut' utility rather than bash's substring expansion notation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-20T07:34:10Z","receivedAt":"2009-08-20T07:34:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland writes:\n\n>> - $sub1sha1 sub1 (${sub1sha1:0:7})\n>> - $sub2sha1 sub2 (${sub1sha1:0:7})\n>> + $sub1sha1 sub1 ($(echo $sub1sha1 | cut -c 1-7))\n>> + $sub2sha1 sub2 ($(echo $sub1sha1 | cut -c 1-7))\n>\n> Typo (both in the original, and the patch), should be:\n> \t$sub2sha1 sub2 ($(echo $sub2sha1 | cut -c 1-7))\n>\n>> $sub3sha1 sub3 (heads/master)\n>> EOF\n>\n> Otherwise:\n>\n> Acked-by: Johan Herland \n\nHmm, what does the command use to shorten these object names? It may be\nsafer and more correct to use \"rev-parse --short\" in case these object\nnames were ambigous in their first 7 hexdigits.\n"},{"id":"121322","messageId":"200908201124.54193.johan@herland.net","threadId":"20669","inReplyTo":"7vskfnq6q5.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH next] t7407: use 'cut' utility rather than bash's substring expansion notation","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2009-08-20T09:24:54Z","receivedAt":"2009-08-20T09:24:54Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Thursday 20 August 2009, Junio C Hamano wrote:\n> Johan Herland writes:\n> >> - $sub1sha1 sub1 (${sub1sha1:0:7})\n> >> - $sub2sha1 sub2 (${sub1sha1:0:7})\n> >> + $sub1sha1 sub1 ($(echo $sub1sha1 | cut -c 1-7))\n> >> + $sub2sha1 sub2 ($(echo $sub1sha1 | cut -c 1-7))\n> >\n> > Typo (both in the original, and the patch), should be:\n> > \t$sub2sha1 sub2 ($(echo $sub2sha1 | cut -c 1-7))\n> >\n> >> $sub3sha1 sub3 (heads/master)\n> >> EOF\n> >\n> > Otherwise:\n> >\n> > Acked-by: Johan Herland \n>\n> Hmm, what does the command use to shorten these object names? It may be\n> safer and more correct to use \"rev-parse --short\" in case these object\n> names were ambigous in their first 7 hexdigits.\n\ngit submodule status (which is what we're testing here) uses\n'git describe' to generate the short object name (see\nset_name_rev in git-submodule.sh). In this case, it falls back\nto 'git describe --all --always', which calls find_unique_abbrev()\non the SHA1. 'git rev-parse --short' ends up calling the same\nfind_unique_abbrev(), so I guess it is better to use it here.\n\nTry this instead:\n\nFrom: Johan Herland \n\nSubject: [PATCH] t7407: Use 'rev-parse --short' rather than bash's substring expansion notation\n\nThe substring expansion notation is a bashism that we have not so far\nadopted. Use 'git rev-parse --short' instead, as this also handles\nthe case where the unique abbreviation is longer than 7 characters.\n\nSuggested-by: Brandon Casey \nSigned-off-by: Johan Herland \n---\n t/t7407-submodule-foreach.sh | 6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh\nindex 25ec281..2a52775 100755\n--- a/t/t7407-submodule-foreach.sh\n+++ b/t/t7407-submodule-foreach.sh\n@@ -201,14 +201,16 @@ submodulesha1=$(cd clone3/nested1/nested2/nested3/submodule && git rev-parse HEA\n sub1sha1=$(cd clone3/sub1 && git rev-parse HEAD)\n sub2sha1=$(cd clone3/sub2 && git rev-parse HEAD)\n sub3sha1=$(cd clone3/sub3 && git rev-parse HEAD)\n+sub1sha1_short=$(cd clone3/sub1 && git rev-parse --short HEAD)\n+sub2sha1_short=$(cd clone3/sub2 && git rev-parse --short HEAD)\n \n cat > expect <\nwww.herland.net\n"},{"id":"121354","messageId":"5cqGRKnaah8NEt1Sx1jHwTRuuDfhT5e-wsLOzZOw4Hf1ady-n-TFfg@cipher.nrlssc.navy.mil","threadId":"20669","inReplyTo":"200908201124.54193.johan@herland.net","subject":"Re: [PATCH next] t7407: use 'cut' utility rather than bash's substring expansion notation","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2009-08-20T16:59:57Z","receivedAt":"2009-08-20T16:59:57Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Johan Herland wrote:\n> On Thursday 20 August 2009, Junio C Hamano wrote:\n>> Johan Herland writes:\n>>>> - $sub1sha1 sub1 (${sub1sha1:0:7})\n>>>> - $sub2sha1 sub2 (${sub1sha1:0:7})\n>>>> + $sub1sha1 sub1 ($(echo $sub1sha1 | cut -c 1-7))\n>>>> + $sub2sha1 sub2 ($(echo $sub1sha1 | cut -c 1-7))\n>>> Typo (both in the original, and the patch), should be:\n>>> \t$sub2sha1 sub2 ($(echo $sub2sha1 | cut -c 1-7))\n>>>\n>>>> $sub3sha1 sub3 (heads/master)\n>>>> EOF\n>>> Otherwise:\n>>>\n>>> Acked-by: Johan Herland \n>> Hmm, what does the command use to shorten these object names? It may be\n>> safer and more correct to use \"rev-parse --short\" in case these object\n>> names were ambigous in their first 7 hexdigits.\n> \n> git submodule status (which is what we're testing here) uses\n> 'git describe' to generate the short object name (see\n> set_name_rev in git-submodule.sh). In this case, it falls back\n> to 'git describe --all --always', which calls find_unique_abbrev()\n> on the SHA1. 'git rev-parse --short' ends up calling the same\n> find_unique_abbrev(), so I guess it is better to use it here.\n> \n> Try this instead:\n> \n> From: Johan Herland \n> \n> Subject: [PATCH] t7407: Use 'rev-parse --short' rather than bash's substring expansion notation\n> \n> The substring expansion notation is a bashism that we have not so far\n> adopted. Use 'git rev-parse --short' instead, as this also handles\n> the case where the unique abbreviation is longer than 7 characters.\n\nWorks for me.\n\nMaybe it should be mentioned that a typo was fixed too.\n\n-brandon\n"}]}