All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Sunshine <sunshine@sunshineco.com>
To: Luke Shumaker <lukeshu@lukeshu.com>
Cc: "Git List" <git@vger.kernel.org>,
	"Avery Pennarun" <apenwarr@gmail.com>,
	"Charles Bailey" <cbailey32@bloomberg.net>,
	"Danny Lin" <danny0838@gmail.com>,
	"David A . Greene" <greened@obbligato.org>,
	"David Aguilar" <davvid@gmail.com>,
	"Jakub Suder" <jakub.suder@gmail.com>,
	"James Denholm" <nod.helm@gmail.com>, "Jeff King" <peff@peff.net>,
	"Jonathan Nieder" <jrnieder@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Roger L Strain" <roger.strain@swri.org>,
	"Techlive Zheng" <techlivezheng@gmail.com>,
	"Luke Shumaker" <lukeshu@datawire.io>
Subject: Re: [PATCH 18/30] subtree: use $* instead of $@ as appropriate
Date: Sat, 24 Apr 2021 01:18:33 -0400	[thread overview]
Message-ID: <CAPig+cQ_7eVgdYuci1JeVwU89VOeOU6kzZ9=yw1ySR9RXzjuTA@mail.gmail.com> (raw)
In-Reply-To: <87zgxoa7cj.wl-lukeshu@lukeshu.com>

On Fri, Apr 23, 2021 at 7:50 PM Luke Shumaker <lukeshu@lukeshu.com> wrote:
> On Fri, 23 Apr 2021 14:40:31 -0600, Eric Sunshine wrote:
> > Nit: I have some trouble following what the commit message is actually
> > trying to say with "smash things" and "separate strings". It might be
> > simpler to say merely that use of "$@" in these particular instances
> > is overkill and possibly misleading to readers not familiar with the
> > finer details of $* vs. "$@".

Oof, I somehow misread the code this patch is touching and ended up
confusing myself, and the confusion bled into my review comments.
Sorry.

> How's this:
> ---
> subtree: use "$*" instead of "$@" as appropriate
>
> "$*" is for when you want to concatenate the args together,
> whitespace-separated; and "$@" is for when you want them to be separate
> strings.
>
> There are several places in subtree that erroneously use $@ when
> concatenating args together into an error message.
>
> For instance, if the args are argv[1]="dead" and argv[2]="beef", then
> the line
>
>     die "You must provide exactly one revision.  Got: '$@'"
>
> surely intends to call 'die' with the argument
>
>     argv[1]="You must provide exactly one revision.  Got: 'dead beef'"
>
> however, because the line used $@ instead of $*, it will actually call
> 'die' with the arguments
>
>     argv[1]="You must provide exactly one revision.  Got: 'dead"
>     argv[2]="beef'"
>
> This isn't a big deal, because 'die' concatenates its arguments together
> anyway (using "$*").  But that doesn't change the fact that it was a
> mistake to use $@ instead of $*, even though in the end $@ still ended
> up doing the right thing.

This explanation spells out the problem nicely.

  reply	other threads:[~2021-04-24  5:19 UTC|newest]

Thread overview: 144+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-23 19:42 [PATCH 00/30] subtree: clean up, improve UX Luke Shumaker
2021-04-23 19:42 ` [PATCH 01/30] .gitignore: Ignore /git-subtree Luke Shumaker
2021-04-23 19:42 ` [PATCH 02/30] subtree: t7900: update for having the default branch name be 'main' Luke Shumaker
2021-04-23 19:42 ` [PATCH 03/30] subtree: t7900: use test-lib.sh's test_count Luke Shumaker
2021-04-23 19:42 ` [PATCH 04/30] subtree: t7900: use consistent formatting Luke Shumaker
2021-04-23 21:51   ` Eric Sunshine
2021-04-23 22:54     ` Luke Shumaker
2021-04-27  7:17     ` Junio C Hamano
2021-04-27 20:41       ` Luke Shumaker
2021-04-28  4:33         ` Junio C Hamano
2021-04-23 19:42 ` [PATCH 05/30] subtree: t7900: comment subtree_test_create_repo Luke Shumaker
2021-04-23 19:42 ` [PATCH 06/30] subtree: t7900: use 'test' for string equality Luke Shumaker
2021-04-23 19:42 ` [PATCH 07/30] subtree: t7900: delete some dead code Luke Shumaker
2021-04-23 19:42 ` [PATCH 08/30] subtree: t7900: fix 'verify one file change per commit' Luke Shumaker
2021-04-23 19:42 ` [PATCH 09/30] subtree: t7900: rename last_commit_message to last_commit_subject Luke Shumaker
2021-04-23 19:42 ` [PATCH 10/30] subtree: t7900: add a test for the -h flag Luke Shumaker
2021-04-23 19:42 ` [PATCH 11/30] subtree: t7900: add porcelain tests for 'pull' and 'push' Luke Shumaker
2021-04-23 20:19   ` Eric Sunshine
2021-04-23 22:27     ` Luke Shumaker
2021-04-23 19:42 ` [PATCH 12/30] subtree: don't have loose code outside of a function Luke Shumaker
2021-04-23 20:05   ` Luke Shumaker
2021-04-23 20:23   ` Eric Sunshine
2021-04-23 22:43     ` Luke Shumaker
2021-04-23 23:11       ` Eric Sunshine
2021-04-23 23:37         ` Luke Shumaker
2021-04-23 19:42 ` [PATCH 13/30] subtree: more consistent error propagation Luke Shumaker
2021-04-23 19:42 ` [PATCH 14/30] subtree: drop support for git < 1.7 Luke Shumaker
2021-04-23 20:07   ` Luke Shumaker
2021-04-23 20:31   ` Eric Sunshine
2021-04-23 23:28     ` Luke Shumaker
2021-04-23 23:50       ` Eric Sunshine
2021-04-24  0:20         ` Luke Shumaker
2021-04-23 19:42 ` [PATCH 15/30] subtree: use `git merge-base --is-ancestor` Luke Shumaker
2021-04-23 19:42 ` [PATCH 16/30] subtree: use git-sh-setup's `say` Luke Shumaker
2021-04-23 19:42 ` [PATCH 17/30] subtree: use more explicit variable names for cmdline args Luke Shumaker
2021-04-23 19:42 ` [PATCH 18/30] subtree: use $* instead of $@ as appropriate Luke Shumaker
2021-04-23 20:40   ` Eric Sunshine
2021-04-23 23:50     ` Luke Shumaker
2021-04-24  5:18       ` Eric Sunshine [this message]
2021-04-23 19:42 ` [PATCH 19/30] subtree: give `$(git --exec-path)` precedence over `$PATH` Luke Shumaker
2021-04-26  8:27   ` =?utf-8?B?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason
2021-04-23 19:42 ` [PATCH 20/30] subtree: use "^{commit}" instead of "^0" Luke Shumaker
2021-04-26  7:43   ` Ævar Arnfjörð Bjarmason
2021-04-23 19:42 ` [PATCH 21/30] subtree: parse revs in individual cmd_ functions Luke Shumaker
2021-04-23 19:42 ` [PATCH 22/30] subtree: remove duplicate check Luke Shumaker
2021-04-23 19:42 ` [PATCH 23/30] subtree: add comments and sanity checks Luke Shumaker
2021-04-23 20:58   ` Eric Sunshine
2021-04-23 23:58     ` Luke Shumaker
2021-04-23 19:42 ` [PATCH 24/30] subtree: don't let debug and progress output clash Luke Shumaker
2021-04-23 21:07   ` Eric Sunshine
2021-04-24  0:44     ` Luke Shumaker
2021-04-24  4:54       ` Eric Sunshine
2021-04-23 19:42 ` [PATCH 25/30] subtree: have $indent actually affect indentation Luke Shumaker
2021-04-23 19:42 ` [PATCH 26/30] subtree: give the docs a once-over Luke Shumaker
2021-04-23 19:42 ` [PATCH 27/30] subtree: allow --squash to be used with --rejoin Luke Shumaker
2021-04-24  5:50   ` Eric Sunshine
2021-04-25  0:04     ` Luke Shumaker
2021-04-23 19:42 ` [PATCH 28/30] subtree: allow 'split' flags to be passed to 'push' Luke Shumaker
2021-04-23 19:42 ` [PATCH 29/30] subtree: push: allow specifying a local rev other than HEAD Luke Shumaker
2021-04-23 19:42 ` [PATCH 30/30] subtree: be stricter about validating flags Luke Shumaker
2021-04-25  2:55   ` Danny Lin
2021-04-26 17:35     ` Luke Shumaker
2021-04-23 20:12 ` [PATCH 00/30] subtree: clean up, improve UX Luke Shumaker
2021-04-26  7:55   ` =?utf-8?B?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason
2021-04-27  7:27   ` Junio C Hamano
2021-04-26 17:44 ` [PATCH v2 " Luke Shumaker
2021-04-26 17:44   ` [PATCH v2 01/30] .gitignore: Ignore /git-subtree Luke Shumaker
2021-04-26 17:44   ` [PATCH v2 02/30] subtree: t7900: update for having the default branch name be 'main' Luke Shumaker
2021-04-26 17:44   ` [PATCH v2 03/30] subtree: t7900: use test-lib.sh's test_count Luke Shumaker
2021-04-26 17:44   ` [PATCH v2 04/30] subtree: t7900: use consistent formatting Luke Shumaker
2021-04-26 19:57     ` Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 05/30] subtree: t7900: comment subtree_test_create_repo Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 06/30] subtree: t7900: use 'test' for string equality Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 07/30] subtree: t7900: delete some dead code Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 08/30] subtree: t7900: fix 'verify one file change per commit' Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 09/30] subtree: t7900: rename last_commit_message to last_commit_subject Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 10/30] subtree: t7900: add a test for the -h flag Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 11/30] subtree: t7900: add porcelain tests for 'pull' and 'push' Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 12/30] subtree: don't have loose code outside of a function Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 13/30] subtree: more consistent error propagation Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 14/30] subtree: drop support for git < 1.7 Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 15/30] subtree: use `git merge-base --is-ancestor` Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 16/30] subtree: use git-sh-setup's `say` Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 17/30] subtree: use more explicit variable names for cmdline args Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 18/30] subtree: use "$*" instead of "$@" as appropriate Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 19/30] subtree: Don't fuss with PATH Luke Shumaker
2021-04-26 23:16     ` Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 20/30] subtree: use "^{commit}" instead of "^0" Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 21/30] subtree: parse revs in individual cmd_ functions Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 22/30] subtree: remove duplicate check Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 23/30] subtree: add comments and sanity checks Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 24/30] subtree: don't let debug and progress output clash Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 25/30] subtree: have $indent actually affect indentation Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 26/30] subtree: give the docs a once-over Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 27/30] subtree: allow --squash to be used with --rejoin Luke Shumaker
2021-04-26 19:58     ` Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 28/30] subtree: allow 'split' flags to be passed to 'push' Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 29/30] subtree: push: allow specifying a local rev other than HEAD Luke Shumaker
2021-04-26 17:45   ` [PATCH v2 30/30] subtree: be stricter about validating flags Luke Shumaker
2021-04-27 21:17   ` [PATCH v3 00/30] subtree: clean up, improve UX Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 01/30] .gitignore: Ignore /git-subtree Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 02/30] subtree: t7900: update for having the default branch name be 'main' Luke Shumaker
2021-04-30  9:38       ` Ævar Arnfjörð Bjarmason
2021-04-30 16:07         ` Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 03/30] subtree: t7900: use test-lib.sh's test_count Luke Shumaker
2021-04-30  9:45       ` Ævar Arnfjörð Bjarmason
2021-04-30 16:10         ` Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 04/30] subtree: t7900: use consistent formatting Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 05/30] subtree: t7900: comment subtree_test_create_repo Luke Shumaker
2021-04-30  9:48       ` Ævar Arnfjörð Bjarmason
2021-04-30 16:13         ` Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 06/30] subtree: t7900: use 'test' for string equality Luke Shumaker
2021-04-30  9:55       ` Ævar Arnfjörð Bjarmason
2021-04-30 16:33         ` Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 07/30] subtree: t7900: delete some dead code Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 08/30] subtree: t7900: fix 'verify one file change per commit' Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 09/30] subtree: t7900: rename last_commit_message to last_commit_subject Luke Shumaker
2021-04-30  9:59       ` Ævar Arnfjörð Bjarmason
2021-04-27 21:17     ` [PATCH v3 10/30] subtree: t7900: add a test for the -h flag Luke Shumaker
2021-04-30 10:01       ` Ævar Arnfjörð Bjarmason
2021-04-30 16:40         ` Luke Shumaker
2021-04-30 12:22       ` Bagas Sanjaya
2021-04-30 16:48         ` Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 11/30] subtree: t7900: add porcelain tests for 'pull' and 'push' Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 12/30] subtree: don't have loose code outside of a function Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 13/30] subtree: more consistent error propagation Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 14/30] subtree: drop support for git < 1.7 Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 15/30] subtree: use `git merge-base --is-ancestor` Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 16/30] subtree: use git-sh-setup's `say` Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 17/30] subtree: use more explicit variable names for cmdline args Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 18/30] subtree: use "$*" instead of "$@" as appropriate Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 19/30] subtree: don't fuss with PATH Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 20/30] subtree: use "^{commit}" instead of "^0" Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 21/30] subtree: parse revs in individual cmd_ functions Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 22/30] subtree: remove duplicate check Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 23/30] subtree: add comments and sanity checks Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 24/30] subtree: don't let debug and progress output clash Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 25/30] subtree: have $indent actually affect indentation Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 26/30] subtree: give the docs a once-over Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 27/30] subtree: allow --squash to be used with --rejoin Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 28/30] subtree: allow 'split' flags to be passed to 'push' Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 29/30] subtree: push: allow specifying a local rev other than HEAD Luke Shumaker
2021-04-27 21:17     ` [PATCH v3 30/30] subtree: be stricter about validating flags Luke Shumaker
     [not found]       ` <CAAgkN4cKUSPKpwuaLG-vR5Z7WFUZ81QXuRcsX-10obaRBAvwBA@mail.gmail.com>
2021-04-28  0:24         ` Luke Shumaker

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='CAPig+cQ_7eVgdYuci1JeVwU89VOeOU6kzZ9=yw1ySR9RXzjuTA@mail.gmail.com' \
    --to=sunshine@sunshineco.com \
    --cc=apenwarr@gmail.com \
    --cc=cbailey32@bloomberg.net \
    --cc=danny0838@gmail.com \
    --cc=davvid@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=greened@obbligato.org \
    --cc=jakub.suder@gmail.com \
    --cc=jrnieder@gmail.com \
    --cc=lukeshu@datawire.io \
    --cc=lukeshu@lukeshu.com \
    --cc=nod.helm@gmail.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=roger.strain@swri.org \
    --cc=techlivezheng@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
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.