Re: [PATCH] Poison gettext with the Ook language

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Mon, Oct 22 2018, Nguyễn Thái Ngọc Duy wrote:

> The current gettext() function just replaces all strings with
> '# GETTEXT POISON #' including format strings and hides the things
> that we should be allowed to grep (like branch names, or some other
> codes) even when gettext is poisoned.
>
> This patch implements the poisoned _() with a universal and totally
> legit language called Ook [1]. We could actually grep stuff even in
> with this because format strings are preserved.
>
> Long term, we could implement an "ook translator" for test_i18ngrep
> and friends so that they translate English to Ook, allowing us to
> match full text while making sure the text in the code is still marked
> for translation.

Replying to both this patch, and SZEDER's series for brevity. Thanks
both for working on this. It's obvious that this stuff needs some
polishing, and it's great that's being done, and sorry for my part of
having left this in the current state.

a)

Both this patch and SZEDER's 7/8 are addressing the issue that the
poison mode doesn't preserve format specifiers.

I haven't tried to dig this up in the list archive, and maybe it only
exists in my mind, but I seem to remember that this was explicitly
discussed as a goal at the time.

I.e. we were expecting the lack of this to break tests, and that was
considered a feature as it would spot plumbing messages we shouldn't be
translating.

However, it's my opinion that this was just a bad idea to begin with,
I've CC'd a couple of prolific markers of i18n from my log grepping (and
feel free to add more) to see if they disagre.

I think it probably helped me a bit in the beginning when i18n was
bootstrapping and there was a *lot* to mark for translation, but we've
long since stabilized and aren't doing that anymore, so it's much easier
to just review the patches to see if they translate plumbing.

All of which is to say that I think something like your patch here is
fine to just accept, and the only improvement I'd suggest is some note
in the commit message saying that this was always intentional, but
nobody can name a case where it helped, so let's just drop it.

In SZEDER's case that we shouldn't have GIT_GETTEXT_POISON=scrambled,
let's just make it boolean and make "scrambled" (i.e. preserving format
specifiecs) the default.

b)

SZEDER's series, and your patch (although it's smaller) still preserve
the notion that there's such a thing as a GETTEXT_POISON *build* and you
actually need to compile git with that flag to make use of it.

This, as I recall, was just paranoia on my part back in 2010/2011. I
wanted to be able to present a version of this i18n stuff where people
who didn't like it could be assured that if they didn't opt-in to it
they wouldn't get any of the code (sans a few trivial and obviously
correct wrapper functions).

So I think the only reason to keep it compile-time is performance, but I
don't think that matters. It's not like we're printing gigabytes of _()
formatted output. Everything where formatting matters is plumbing which
doesn't use this API. These messages are always for human consumption.

So shouldn't we just drop this notion of GETTEXT_POISON at compile-time
entirely, and make GIT_GETTEXT_POISON=<boolean> a test flag that all
builds of git are aware of? I think so.