On Fri, Apr 23, 2021 at 7:50 PM Luke Shumaker <lukeshu@xxxxxxxxxxx> 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.
