Re: [PATCH v3 3/5] commit test: Use write_script

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, May 26, 2014 at 2:56 PM, Caleb Thompson <cjaysson@xxxxxxxxx> wrote:
> Use write_script from t/test-lib-functions instead of cat, shebang, and
> chmod.
>
> Signed-off-by: Caleb Thompson <caleb@xxxxxxxxxxxxxxxx>
> ---
>  t/t7507-commit-verbose.sh | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
> index 3b06d73..e62d921 100755
> --- a/t/t7507-commit-verbose.sh
> +++ b/t/t7507-commit-verbose.sh
> @@ -3,11 +3,9 @@
>  test_description='verbose commit template'
>  . ./test-lib.sh
>
> -cat >check-for-diff <<EOF
> -#!$SHELL_PATH
> -exec grep '^diff --git' "\$1"
> +write_script check-for-diff <<-EOF
> +       exec grep '^diff --git' "\$1"

Food for thought:

The original code used <<EOF since it needed $SHELL_PATH to be
evaluated at script creation time, and took special care to escape $1
in the 'grep' invocation since $1 should be evaluated only at script
execution time.

With the change to write_script(), nothing within the here-doc
requires evaluation, yet you are still using the evaluating <<-EOF
form (and manually escaping $1). The intent might be clearer if you
switch to <<-\EOF which suppresses evaluation (and drop the manual
escaping of $1).

The same observation applies to the new write_script() invocation to
create check-for-no-diff in patch 5.

>  EOF
> -chmod +x check-for-diff
>  test_set_editor "$(pwd)/check-for-diff"
>
>  cat >message <<'EOF'
> --
> 1.9.3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]