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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: May 27, 2014, 22:30 UTC
Message-ID: <CAPig+cQt0mfBTChw8y=2Jg3rNsSr+neDCresptBafPDQixseXA@mail.gmail.com>
In-Reply-To: <1401130586-93105-4-git-send-email-caleb@calebthompson.io>

On Mon, May 26, 2014 at 2:56 PM, Caleb Thompson <cjaysson@gmail.com> wrote:

Show 21 quoted lines

> Use write_script from t/test-lib-functions instead of cat, shebang, and
> chmod.
>
> Signed-off-by: Caleb Thompson <caleb@calebthompson.io>
> ---
>  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.

Show 7 quoted lines

>  EOF
> -chmod +x check-for-diff
>  test_set_editor "$(pwd)/check-for-diff"
>
>  cat >message <<'EOF'
> --
> 1.9.3