git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Chris Torek <chris.torek@gmail.com>
Cc: "Martin Ågren" <martin.agren@gmail.com>,
	"Git List" <git@vger.kernel.org>,
	"Jonathan Tan" <jonathantanmy@google.com>
Subject: Re: [PATCH] t1450: fix quoting of NUL byte when corrupting pack
Date: Sun, 02 Aug 2020 10:57:40 -0700
Message-ID: <xmqqmu3drkvf.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <CAPx1GvdZNeuQqmYm8G62Zr02k=B5GK69xPw84WnvMCeJU7_amQ@mail.gmail.com> (Chris Torek's message of "Sun, 2 Aug 2020 09:20:13 -0700")

Chris Torek <chris.torek@gmail.com> writes:

> On Sun, Aug 2, 2020 at 7:35 AM Martin Ågren <martin.agren@gmail.com> wrote:
>> No worries! Thanks for having a look at the patch. Is there anything
>> that could be done to make this clearer in the commit message? (I find it
>> quite awkward to discuss quoting: will the reader understand which
>> quoting is part of my own formatting of the message vs which is part of
>> the quoting issue I want to get across!?)
>
> This is indeed a problem...
>
> Perhaps something along these lines (generic boilerplate
> for any single-quote fixes, that should be adjusted for the
> actual fix):
>
>     In the test scripts, the recommended style is, e.g.:
>
>         test_expect_success 'name' '
>             multi-line test
>             goes here
>         '
>
>     When using this style, any single quote in the multi-line
>     test section is actually closing the lone single quotes
>     that surround it.  To avoid confusion, minimize and/or
>     eliminate the use of single quotes here.

Another thing that falls into the same class and probably be a good
addition to the above "tip" is how $variables are interpolated, i.e.

	test_expect_success 'test name' '
		test-that-references $variable &&
		another-test-that-references "$variable"
	'

are 99% of the time the right way to refer to variable that is
assigned outside the test itself (e.g. the whole four lines shown
above may be in a loop, "for variable in a b c; ... ;done").

	test_expect_success 'test name' '
		test-that-references '$variable' &&
		another-test-that-references '"$variable"'
	'

is most likely a wrong way to write for the first one (i.e. what if
the value in $variable has $IFS whitespace) and "not wrong per-se
but unnecessary" for the second one.

Same applies to $(command) substitution, but it is more important.
"Step out of the quote, evaluate and step back into the quote"
pattern would mean the command is evaluated while formulating the
body of the test, not while running the test, which often is not
what the author intended.

Thanks.

      reply	other threads:[~2020-08-02 17:57 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-01 22:06 Martin Ågren
2020-08-02  0:45 ` Junio C Hamano
2020-08-02 14:30   ` Martin Ågren
2020-08-02 17:22     ` Eric Sunshine
2020-08-06 20:08     ` [PATCH v2 0/2] t: don't spuriously close and reopen quotes Martin Ågren
2020-08-06 20:08       ` [PATCH v2 1/2] " Martin Ågren
2020-08-06 20:26         ` Eric Sunshine
2020-08-07  8:45           ` Martin Ågren
2020-08-07 16:17             ` Eric Sunshine
2020-08-07 17:16               ` Junio C Hamano
2020-08-06 20:08       ` [PATCH v2 2/2] t4104: modernize and simplify quoting Martin Ågren
2020-08-02  1:00 ` [PATCH] t1450: fix quoting of NUL byte when corrupting pack Chris Torek
2020-08-02  1:02   ` Chris Torek
2020-08-02 14:35     ` Martin Ågren
2020-08-02 16:20       ` Chris Torek
2020-08-02 17:57         ` Junio C Hamano [this message]

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqmu3drkvf.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=chris.torek@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.com \
    --cc=martin.agren@gmail.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

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ http://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for the project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git