From: Junio C Hamano <email@example.com> To: Chris Torek <firstname.lastname@example.org> Cc: "Martin Ågren" <email@example.com>, "Git List" <firstname.lastname@example.org>, "Jonathan Tan" <email@example.com> Subject: Re: [PATCH] t1450: fix quoting of NUL byte when corrupting pack Date: Sun, 02 Aug 2020 10:57:40 -0700 Message-ID: <firstname.lastname@example.org> (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 <email@example.com> writes: > On Sun, Aug 2, 2020 at 7:35 AM Martin Ågren <firstname.lastname@example.org> 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.
prev parent reply index 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 * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ /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 Mailing List Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/git/0 git/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 git git/ https://lore.kernel.org/git \ email@example.com public-inbox-index git Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.git AGPL code for this site: git clone https://public-inbox.org/public-inbox.git