Re: [PATCH] Poison gettext with the Ook language

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Oct 22, 2018, 21:09 UTC
Message-ID: <878t2pd6yu.fsf@evledraar.gmail.com>
In-Reply-To: <20181022153633.31757-1-pclouds@gmail.com>

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

Show 13 quoted lines

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