From: Junio C Hamano <gitster@pobox.com>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: AbdAlRahman Gad <abdobngad@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH v4 8/8] [Newcomer] t7004: Use single quotes instead of double quotes
Date: Tue, 06 Aug 2024 09:35:33 -0700 [thread overview]
Message-ID: <xmqqo765vcbu.fsf@gitster.g> (raw)
In-Reply-To: <CAPig+cSzq+6bua=T7w0M-+7+2zbzfD6ihXLkWfondZ8NbqUG4A@mail.gmail.com> (Eric Sunshine's message of "Mon, 5 Aug 2024 23:02:01 -0400")
Eric Sunshine <sunshine@sunshineco.com> writes:
> As a practical example, consider a test with a body like this:
>
> echo nothing >nothing &&
> git add nothing &&
> git commit -m nothing &&
> hash=$(git rev-parse HEAD) &&
> ...
>
> If this body is inside a double-quoted string, then `$(git rev-parse
> HEAD)` will be evaluated and its value assigned to `hash` _before_
> test_expect_success() is called,
I know it is just your finger slipping, but the variable "hash" is
not assigned to before test_expect_success is called even with the
body inside dq.
What happens is that the value of HEAD is expanded in the string
that will be evaled by test_expect_success so the 4th line in the
above becomes "hash=3469a23659d8197190d2765cf9f31dec5ab602fa &&";
as the resulting string is then eval'ed by test_expect_success,
the end result is as you descirbed, i.e., ...
> thus also before the `git commit`
> command inside the test body (which is almost certainly not what the
> author intended).
... $hash does not get the name of the commit object resulting from
the "git commit" command before it.
>> - "
>> - test_expect_success "Doing 'git tag --list-like $option <commit> <pattern> is permitted" "
>> + '
>> + test_expect_success 'Doing "git tag --list-like $option <commit> <pattern> is permitted' '
>
> ... changing the double-quotes to single-quotes for the test _titles_
> in these instances is actively wrong. In this case, we _want_
> interpolation of `$option` to happen in the title string so that the
> output looks like this:
>
> ok 167 - mixing incompatible modes with --contains is forbidden
> ok 169 - mixing incompatible modes with --with is forbidden
> ok 171 - mixing incompatible modes with --no-contains is forbidden
>
> By changing the title to use single-quotes, you suppress interpolation
> of `$option`, with the result that the displayed titles become rather
> useless:
>
> ok 167 - mixing incompatible modes with $option is forbidden
> ok 169 - mixing incompatible modes with $option is forbidden
> ok 171 - mixing incompatible modes with $option is forbidden
Yes, these has to be done carefully, both for titles and bodies.
Thanks.
next prev parent reply other threads:[~2024-08-06 16:35 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-05 23:59 [PATCH v4 0/8] t7004: Modernize the style AbdAlRahman Gad
2024-08-05 23:59 ` [PATCH v4 1/8] [Newcomer] t7004: Remove space after redirect operators AbdAlRahman Gad
2024-08-05 23:59 ` [PATCH v4 2/8] [Newcomer] t7004: Do not lose exit status to pipe AbdAlRahman Gad
2024-08-06 3:13 ` Eric Sunshine
2024-08-06 8:38 ` AbdAlRahman Gad
2024-08-06 18:05 ` Eric Sunshine
2024-08-05 23:59 ` [PATCH v4 3/8] [Newcomer] t7004: One command per line AbdAlRahman Gad
2024-08-05 23:59 ` [PATCH v4 4/8] [Newcomer] t7004: Do not prepare things outside test_expect_success AbdAlRahman Gad
2024-08-06 3:29 ` Eric Sunshine
2024-08-06 8:41 ` AbdAlRahman Gad
2024-08-05 23:59 ` [PATCH v4 5/8] [Newcomer] t7004: Use indented here-doc AbdAlRahman Gad
2024-08-05 23:59 ` [PATCH v4 6/8] [Newcomer] t7004: Description on the same line as test_expect_success AbdAlRahman Gad
2024-08-05 23:59 ` [PATCH v4 7/8] [Newcomer] t7004: test Description and test body seperated with backslash AbdAlRahman Gad
2024-08-05 23:59 ` [PATCH v4 8/8] [Newcomer] t7004: Use single quotes instead of double quotes AbdAlRahman Gad
2024-08-06 3:02 ` Eric Sunshine
2024-08-06 10:10 ` AbdAlRahman Gad
2024-08-06 17:21 ` Eric Sunshine
2024-08-06 16:35 ` Junio C Hamano [this message]
2024-08-06 17:11 ` Eric Sunshine
2024-08-06 0:45 ` [PATCH v4 0/8] t7004: Modernize the style Junio C Hamano
2024-08-06 8:44 ` AbdAlRahman Gad
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=xmqqo765vcbu.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=abdobngad@gmail.com \
--cc=git@vger.kernel.org \
--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.