Re: [PATCH 09/11] GETTEXT_POISON=rot13: do compare the output in `test_i18ncmp`

From: Junio C Hamano <gitster@pobox.com>
To: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Ævar Arnfjörð" <avarab@gmail.com>,
	"SZEDER Gábor" <szeder.dev@gmail.com>,
	"Johannes Schindelin" <johannes.schindelin@gmx.de>
Subject: Re: [PATCH 09/11] GETTEXT_POISON=rot13: do compare the output in `test_i18ncmp`
Date: Tue, 12 Jan 2021 11:47:28 -0800	[thread overview]
Message-ID: <xmqqeeiq3pkf.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <4669ccbb1ae40f0c58a2d8e3c8b3a34d82176c7a.1610441263.git.gitgitgadget@gmail.com> (Johannes Schindelin via GitGitGadget's message of "Tue, 12 Jan 2021 08:47:40 +0000")

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> +static size_t unrot13(char *buf)
> +{
> +	char *p = buf, *q = buf;
> +
> +	while (*p) {
> +		const char *begin = strstr(p, "<rot13>"), *end;

AFAIR from my reading of [02/11], the encoding side did not special
case the payload that has <ebg13> or </ebg13>; if we want to make it
reversible conversion (which is excellent improvement over the
current "# GETTEXT_POISON #" obfuscation), we'd need to do something
about it, I think.

But on second thought, nobody can prevent a caller to die(_("%s", msg));
to have "<rot13>" in the msg part, so perhaps punting like this
series does is sufficient.  I dunno.

> +		if (!begin)
> +			break;
> +
> +		while (p != begin)
> +			*(q++) = *(p++);
> +
> +		p += strlen("<rot13>");
> +		end = strstr(p, "</rot13>");
> +		if (!end)
> +			BUG("could not find </rot13> in\n%s", buf);

And the user of this looks quite straightforward and nice.

>  test_i18ncmp () {
> -	! test_have_prereq C_LOCALE_OUTPUT || test_cmp "$@"
> +	if test rot13 = "$GIT_TEST_GETTEXT_POISON"
> +	then
> +		test-tool i18n cmp "$@"
> +	elif test_have_prereq C_LOCALE_OUTPUT
> +	then
> +		test_cmp "$@"
> +	fi
>  }