From: Junio C Hamano <gitster@pobox.com>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: Caleb Thompson <cjaysson@gmail.com>,
Git List <git@vger.kernel.org>, Jeff King <peff@peff.net>,
Jeremiah Mahler <jmmahler@gmail.com>,
Duy Nguyen <pclouds@gmail.com>
Subject: Re: [PATCH v3 3/5] commit test: Use write_script
Date: Tue, 27 May 2014 15:42:23 -0700 [thread overview]
Message-ID: <xmqqoayietdc.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CAPig+cQt0mfBTChw8y=2Jg3rNsSr+neDCresptBafPDQixseXA@mail.gmail.com> (Eric Sunshine's message of "Tue, 27 May 2014 18:30:42 -0400")
Eric Sunshine <sunshine@sunshineco.com> writes:
>> -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.
Very good comments. Thanks.
next prev parent reply other threads:[~2014-05-27 22:42 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-25 6:24 [PATCH v2] commit: support commit.verbose and --no-verbose Caleb Thompson
2014-05-25 7:02 ` Jeremiah Mahler
2014-05-25 7:44 ` Jeremiah Mahler
2014-05-25 8:44 ` Duy Nguyen
2014-05-25 10:23 ` Eric Sunshine
2014-05-26 18:56 ` [PATCH v3 0/5] " Caleb Thompson
2014-05-26 18:56 ` [PATCH v3 1/5] commit test: Use test_config instead of git-config Caleb Thompson
2014-05-26 18:56 ` [PATCH v3 2/5] commit test: Change $PWD to $(pwd) Caleb Thompson
2014-05-27 5:46 ` Johannes Sixt
2014-05-27 6:10 ` Eric Sunshine
2014-05-27 6:14 ` Jeremiah Mahler
2014-05-27 6:34 ` Johannes Sixt
2014-05-27 7:35 ` David Kastrup
2014-05-26 18:56 ` [PATCH v3 3/5] commit test: Use write_script Caleb Thompson
2014-05-27 22:30 ` Eric Sunshine
2014-05-27 22:42 ` Junio C Hamano [this message]
2014-05-26 18:56 ` [PATCH v3 4/5] commit test: test_set_editor in each test Caleb Thompson
2014-05-27 22:59 ` Eric Sunshine
2014-05-26 18:56 ` [PATCH v3 5/5] commit: support commit.verbose and --no-verbose Caleb Thompson
2014-05-26 20:33 ` Jeremiah Mahler
2014-05-26 20:47 ` Caleb Thompson
[not found] ` <CA+g4mq8iGNVm-2Uj8j2bJLDazaTS_U76BO9-jeS9Aw4RZnki5A@mail.gmail.com>
2014-05-26 21:00 ` Jeremiah Mahler
2014-05-26 22:14 ` Jeremiah Mahler
2014-05-26 22:34 ` [PATCH v3 0/5] " Jeremiah Mahler
2014-05-26 22:40 ` Caleb Thompson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqoayietdc.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox.com \
--cc=cjaysson@gmail.com \
--cc=git@vger.kernel.org \
--cc=jmmahler@gmail.com \
--cc=pclouds@gmail.com \
--cc=peff@peff.net \
--cc=sunshine@sunshineco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.