On Fri, Apr 23, 2021 at 7:50 PM Luke Shumaker [off-list ref] wrote:

On Fri, 23 Apr 2021 14:40:31 -0600, Eric Sunshine wrote:

quoted

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.