Subject: Re: [PATCH 1/2] sha1-array: add test-sha1-array and basic tests

From: Jeff King <[email-masked]>
Date: Wed, 1 Oct 2014 10:23:36 -0400

On Wed, Oct 01, 2014 at 10:11:04AM -0400, Eric Sunshine wrote:

> > +echo20() {
> > +       prefix="$1"
> > +       shift
> > +       while test $# -gt 0
> > +       do
> > +               echo "$prefix$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1"
> 
> Each caller of echo20() manually includes a space at the end of
> $prefix. Would it make sense to instead have echo20() do this on
> behalf of the caller?
> 
>     echo "$prefix $1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1"

Not always. For example:

> > +test_expect_success 'ordered enumeration' '
> > +       echo20 "" 44 55 88 aa >expect &&

This does not.

> > +       {
> > +               echo20 "append " 88 44 aa 55 &&
> 
> Which would slightly reduce the burden on the caller and make it read
> (very slightly) nicer:
> 
>     echo20 append 88 44 aa 55 &&

I agree that is more readable. But you'd have to make echo20 more like:

  if test -n "$1"; then
          prefix="$1 "
  else
          prefix=""
  fi

which is not too bad.

-Peff