Re: read test snippet from stdin [was: [PATCH] t3900: add some more quotes]

From: Jeff King <peff@peff.net>
Date: Jan 11, 2018, 12:11 UTC
Message-ID: <20180111121143.GA6357@sigill.intra.peff.net>
In-Reply-To: <20180111113928.6412-1-szeder.dev@gmail.com>

On Thu, Jan 11, 2018 at 12:39:28PM +0100, SZEDER Gábor wrote:

Show 19 quoted lines

> > diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh
> > index 9e4e694d93..09ad4d8878 100755
> > --- a/t/t3900-i18n-commit.sh
> > +++ b/t/t3900-i18n-commit.sh
> > @@ -39,14 +39,14 @@ test_expect_success 'UTF-16 refused because of NULs' '
> >  	test_must_fail git commit -a -F "$TEST_DIRECTORY"/t3900/UTF-16.txt
> >  '
> >  
> > -test_expect_success 'UTF-8 invalid characters refused' '
> 
> Note that the test snippet started right after that last single quote,
> i.e. it started with a newline.
> 
> > -	test_when_finished "rm -f \"$HOME/stderr $HOME/invalid\"" &&
> > +test_expect_success 'UTF-8 invalid characters refused' - <<\EOT
> > +	test_when_finished 'rm -f "$HOME/stderr $HOME/invalid"' &&
> 
> And now it starts at the beginning of this line, i.e. without that
> leading neline.  This change leads to the following when run with '-v':

Yeah, I noticed that, too. It would be easy enough to add the extra newline ourselves when printing the verbose output (quite a few of our older tests don't start with a newline already, so it would be improving them, too).

Alternatively, I considered adding a whole new helper function that would skip the need to say "-" as the test body. And then it would always know we were reading from the here-doc.

Show 9 quoted lines

> > +		# command substitution eats trailing whitespace, so we add
> > +		# and then remove a non-whitespace character.
> > +		eval "$1=\$(cat; printf x)"
> > +		eval "$1=\${$1%x}"
> > +	fi
> > +}
> 
> Command substitutions don't eat trailing whitespaces in general, only
> trailing newlines.  POSIX:

Yeah, I wondered about that, but didn't bother digging since the solution is the same either way.

Show 8 quoted lines

> How about this alternative (also adding the missing leading newline
> mentioned above):
> 
> +		eval "$1='
> +'\$(cat)'
> +'"
> 
> The indentation is yuck, but overall perhaps a bit less hacky...

I wrote something very similar at first, before finding the printf trick on Stack Overflow. I thought the indentation on what I wrote was too ugly. :)

I don't have a strong preference, and certainly if one is more portable than the other, we should choose that.

The main point of my email was just to say "do people even like the concept/direction?"

-Peff