git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: "SZEDER Gábor" <szeder.dev@gmail.com>
Cc: Johannes Sixt <j6t@kdbg.org>, Beat Bolli <dev+git@drbeat.li>,
	git@vger.kernel.org,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: read test snippet from stdin [was: [PATCH] t3900: add some more quotes]
Date: Thu, 11 Jan 2018 07:11:44 -0500	[thread overview]
Message-ID: <20180111121143.GA6357@sigill.intra.peff.net> (raw)
In-Reply-To: <20180111113928.6412-1-szeder.dev@gmail.com>

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

> > 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.

> > +		# 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.

> 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

      reply	other threads:[~2018-01-11 12:11 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-10  9:58 [PATCH] t3900: add some more quotes Beat Bolli
2018-01-10 10:33 ` Jeff King
2018-01-10 17:21   ` Johannes Schindelin
2018-01-10 17:44 ` Eric Sunshine
2018-01-10 22:38   ` [PATCH v2] " Beat Bolli
2018-01-10 23:09     ` Johannes Schindelin
2018-01-10 23:09     ` Junio C Hamano
2018-01-10 19:02 ` [PATCH] " Johannes Sixt
2018-01-10 19:43   ` Junio C Hamano
2018-01-10 19:53   ` Jeff King
2018-01-10 21:31     ` Junio C Hamano
2018-01-11  9:38       ` Jeff King
2018-01-11 11:39     ` read test snippet from stdin [was: [PATCH] t3900: add some more quotes] SZEDER Gábor
2018-01-11 12:11       ` Jeff King [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180111121143.GA6357@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=dev+git@drbeat.li \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=szeder.dev@gmail.com \
    --subject='Re: read test snippet from stdin [was: [PATCH] t3900: add some more quotes]' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).