Re: [PATCH 07/10] t5100-mailinfo.sh: use the $( ... ) construct for command substitution

[Junio C Hamano]

Junio C Hamano  writes:

> Very true.  Let's leave that kind of things as separate clean-ups
> after these patches settle, as mixing manual and mechanical changes
> in a single patch makes it harder to review.
>
> Thanks.

So that I won't forget (I'll need to amend with your sign-off even
if this one is satisfactory to you).

-- >8 --
From: Johannes Sixt 
Date: Tue, 22 Dec 2015 19:35:16 +0100
Subject: [PATCH] t5100: no need to use 'echo' command substitutions for globbing

Instead of making the shell expand 00* and invoke 'echo' with it,
and then capturing its output as command substitution, just use
the result of expanding 00* directly.

Signed-off-by: Junio C Hamano 
---
 t/t5100-mailinfo.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh
index 4e52b3b..85b3df5 100755
--- a/t/t5100-mailinfo.sh
+++ b/t/t5100-mailinfo.sh
@@ -23,7 +23,7 @@ check_mailinfo () {
 }
 
 
-for mail in $(echo 00*)
+for mail in 00*
 do
test_expect_success "mailinfo $mail" '
check_mailinfo $mail "" &&
@@ -51,7 +51,7 @@ test_expect_success 'split box with rfc2047 samples' \
echo total is $last &&
test $(cat rfc2047/last) = 11'
 
-for mail in $(echo rfc2047/00*)
+for mail in rfc2047/00*
 do
test_expect_success "mailinfo $mail" '
git mailinfo -u $mail-msg $mail-patch <$mail >$mail-info &&
-- 
2.7.0-rc2-145-g5695473
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/10] t5100-mailinfo.sh: use the $( ... ) construct for command substitution

[Junio C Hamano]

Johannes Sixt  writes:

> Am 22.12.2015 um 19:35 schrieb Johannes Sixt:
>> Am 22.12.2015 um 16:27 schrieb Elia Pinto:
>>> -for mail in `echo 00*`
>>> +for mail in $(echo 00*)
>>
>>> -for mail in `echo rfc2047/00*`
>>> +for mail in $(echo rfc2047/00*)
>>
>> True, these are equvalence transformations. But a better way to get rid
>> of the back-quotes is to write these lines as
>>
>> for mail in echo 00*
>> for mail in echo rfc2047/00*
>
> Ahem... both of these lines without the 'echo', of course!

Very true.  Let's leave that kind of things as separate clean-ups
after these patches settle, as mixing manual and mechanical changes
in a single patch makes it harder to review.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/10] t5100-mailinfo.sh: use the $( ... ) construct for command substitution

[Johannes Sixt]

Am 28.12.2015 um 02:59 schrieb Junio C Hamano:

Junio C Hamano  writes:


Very true.  Let's leave that kind of things as separate clean-ups
after these patches settle, as mixing manual and mechanical changes
in a single patch makes it harder to review.

Thanks.


So that I won't forget (I'll need to amend with your sign-off even
if this one is satisfactory to you).

-- >8 --
From: Johannes Sixt 
Date: Tue, 22 Dec 2015 19:35:16 +0100
Subject: [PATCH] t5100: no need to use 'echo' command substitutions for globbing

Instead of making the shell expand 00* and invoke 'echo' with it,
and then capturing its output as command substitution, just use
the result of expanding 00* directly.

Signed-off-by: Junio C Hamano 
---
  t/t5100-mailinfo.sh | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh
index 4e52b3b..85b3df5 100755
--- a/t/t5100-mailinfo.sh
+++ b/t/t5100-mailinfo.sh
@@ -23,7 +23,7 @@ check_mailinfo () {
  }


-for mail in $(echo 00*)
+for mail in 00*
  do
test_expect_success "mailinfo $mail" '
check_mailinfo $mail "" &&
@@ -51,7 +51,7 @@ test_expect_success 'split box with rfc2047 samples' \
echo total is $last &&
test $(cat rfc2047/last) = 11'

-for mail in $(echo rfc2047/00*)
+for mail in rfc2047/00*
  do
test_expect_success "mailinfo $mail" '
git mailinfo -u $mail-msg $mail-patch <$mail >$mail-info &&



Signed-off-by: Johannes Sixt 

Thank you for making a proper patch!

-- Hannes

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 07/10] t5100-mailinfo.sh: use the $( ... ) construct for command substitution

[Elia Pinto]

The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.

The backquoted form is the traditional method for command
substitution, and is supported by POSIX.  However, all but the
simplest uses become complicated quickly.  In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.

The patch was generated by:

for _f in $(find . -name "*.sh")
do
perl -i -pe 'BEGIN{undef $/;} s/`(.+?)`/\$(\1)/smg'  "${_f}"
done

and then carefully proof-read.

Signed-off-by: Elia Pinto 
---
 t/t5100-mailinfo.sh | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh
index e97cfb2..4e52b3b 100755
--- a/t/t5100-mailinfo.sh
+++ b/t/t5100-mailinfo.sh
@@ -9,9 +9,9 @@ test_description='git mailinfo and git mailsplit test'
 
 test_expect_success 'split sample box' \
'git mailsplit -o. "$TEST_DIRECTORY"/t5100/sample.mbox >last &&
-   last=`cat last` &&
+   last=$(cat last) &&
echo total is $last &&
-   test `cat last` = 17'
+   test $(cat last) = 17'
 
 check_mailinfo () {
mail=$1 opt=$2
@@ -23,7 +23,7 @@ check_mailinfo () {
 }
 
 
-for mail in `echo 00*`
+for mail in $(echo 00*)
 do
test_expect_success "mailinfo $mail" '
check_mailinfo $mail "" &&
@@ -47,11 +47,11 @@ test_expect_success 'split box with rfc2047 samples' \
'mkdir rfc2047 &&
git mailsplit -orfc2047 "$TEST_DIRECTORY"/t5100/rfc2047-samples.mbox \
  >rfc2047/last &&
-   last=`cat rfc2047/last` &&
+   last=$(cat rfc2047/last) &&
echo total is $last &&
-   test `cat rfc2047/last` = 11'
+   test $(cat rfc2047/last) = 11'
 
-for mail in `echo rfc2047/00*`
+for mail in $(echo rfc2047/00*)
 do
test_expect_success "mailinfo $mail" '
git mailinfo -u $mail-msg $mail-patch <$mail >$mail-info &&
-- 
2.3.3.GIT

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/10] t5100-mailinfo.sh: use the $( ... ) construct for command substitution

[Johannes Sixt]

Am 22.12.2015 um 16:27 schrieb Elia Pinto:

-for mail in `echo 00*`
+for mail in $(echo 00*)



-for mail in `echo rfc2047/00*`
+for mail in $(echo rfc2047/00*)


True, these are equvalence transformations. But a better way to get rid 
of the back-quotes is to write these lines as


for mail in echo 00*
for mail in echo rfc2047/00*

No?

-- Hannes

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/10] t5100-mailinfo.sh: use the $( ... ) construct for command substitution

[Johannes Sixt]

Am 22.12.2015 um 19:35 schrieb Johannes Sixt:

Am 22.12.2015 um 16:27 schrieb Elia Pinto:

-for mail in `echo 00*`
+for mail in $(echo 00*)



-for mail in `echo rfc2047/00*`
+for mail in $(echo rfc2047/00*)


True, these are equvalence transformations. But a better way to get rid
of the back-quotes is to write these lines as

for mail in echo 00*
for mail in echo rfc2047/00*


Ahem... both of these lines without the 'echo', of course!



No?

-- Hannes



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html