On Fri, Jun 28, 2019 at 05:32:35PM -0400, Eric Sunshine wrote:
> > +# The message, filename, and contents strings are evaluated by the shell
> > inside
> > +# double-quotes, with $n set to the current commit number. So you can do:
> > +#
> > +# test_commit_bulk --filename=file --contents='modification $n'
>
> Considering that test_commit_bulk() is intended to be used within a
> test body, and considering that test bodies are almost always
> encapsulated in single quotes, recommending single quoting the value
> of --contents= seems contraindicated. Double quotes likely would be
> better.
I hoped people reading it would realize that they would need to suppress
interpolation of $ one way or another. But I guess many people who touch
the test suite aren't actually prolific shell writers.
Anyway, I'm going to look into changing this to a printf string, which
would make this easier.
> > + in_dir=${indir:+-C "$indir"}
>
> Doesn't this suffer the problem in which some older/broken
> shells[1][2][3][4] incorrectly expand this to:
That line is leftover dead code; see my response to Junio.
> Same comment applies to other instances of ${indir:+-C "$indir"} below.
Those ones are fine because of the double-quotes (and whitespace
splitting on the replacement value happens before interpolation). Try
this:
x=''
y='with spaces'
sh -c 'for i in "$@"; do echo arg: $i; done' -- \
${x:+-x "$x"} \
${y:+-y "$y"}
-Peff
