From: "Ævar Arnfjörð Bjarmason" <firstname.lastname@example.org> To: "Nguyễn Thái Ngọc Duy" <email@example.com> Cc: firstname.lastname@example.org, "SZEDER Gábor" <email@example.com>, "Vasco Almeida" <firstname.lastname@example.org>, "Jiang Xin" <email@example.com> Subject: Re: [PATCH] Poison gettext with the Ook language Date: Mon, 22 Oct 2018 23:09:13 +0200 Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <email@example.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 . 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.
next prev parent reply index Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-10-22 15:36 Nguyễn Thái Ngọc Duy 2018-10-22 20:22 ` SZEDER Gábor 2018-10-22 20:22 ` [PATCH 1/8] test-lib.sh: preserve GIT_GETTEXT_POISON from the environment SZEDER Gábor 2018-10-22 20:22 ` [PATCH 2/8] gettext: don't poison if GIT_GETTEXT_POISON is set but empty SZEDER Gábor 2018-10-22 20:38 ` Ævar Arnfjörð Bjarmason 2018-10-22 20:22 ` [PATCH 3/8] lib-rebase: loosen GETTEXT_POISON check in fake editor SZEDER Gábor 2018-10-22 20:22 ` [PATCH 4/8] gettext: #ifdef away GETTEXT POISON-related code from _() and Q_() SZEDER Gábor 2018-10-22 20:22 ` [PATCH 5/8] gettext: put "# GETTEXT POISON #" string literal into a macro SZEDER Gábor 2018-10-22 20:22 ` [PATCH 6/8] gettext: use an enum for the mode of GETTEXT POISONing SZEDER Gábor 2018-10-22 20:22 ` [PATCH 7/8] gettext: introduce GIT_GETTEXT_POISON=scrambled SZEDER Gábor 2018-10-23 14:44 ` Duy Nguyen 2018-10-22 20:22 ` [PATCH 8/8] travis-ci: run GETTEXT POISON build job in scrambled mode, too SZEDER Gábor 2018-10-23 14:37 ` [PATCH] Poison gettext with the Ook language Duy Nguyen 2018-10-27 10:11 ` Jakub Narebski 2018-10-22 20:52 ` Johannes Schindelin 2018-10-22 21:09 ` Ævar Arnfjörð Bjarmason [this message] 2018-10-22 21:46 ` Ævar Arnfjörð Bjarmason 2018-10-22 23:04 ` Junio C Hamano 2018-10-23 21:01 ` [PATCH] i18n: make GETTEXT_POISON a runtime option Ævar Arnfjörð Bjarmason 2018-10-24 5:45 ` Junio C Hamano 2018-10-24 7:44 ` Jeff King 2018-10-25 1:00 ` Junio C Hamano 2018-10-25 1:09 ` Jeff King 2018-10-25 1:24 ` Ramsay Jones 2018-10-25 21:23 ` Jeff King 2018-10-26 19:20 ` Ævar Arnfjörð Bjarmason 2018-10-27 6:59 ` Jeff King 2018-10-27 10:42 ` Ævar Arnfjörð Bjarmason 2018-10-24 11:47 ` [PATCH v2] " Ævar Arnfjörð Bjarmason 2018-11-01 19:31 ` [PATCH v3] " Ævar Arnfjörð Bjarmason 2018-11-02 3:11 ` Junio C Hamano 2018-11-02 16:37 ` SZEDER Gábor 2018-11-08 20:26 ` Ævar Arnfjörð Bjarmason 2018-11-08 20:51 ` SZEDER Gábor 2018-11-08 3:24 ` Junio C Hamano 2018-11-08 19:12 ` Eric Sunshine 2018-11-08 21:15 ` [PATCH v4 0/2] " Ævar Arnfjörð Bjarmason 2018-11-08 21:15 ` [PATCH v4 1/2] " Ævar Arnfjörð Bjarmason 2018-11-08 21:15 ` [PATCH v4 2/2] Makefile: ease dynamic-gettext-poison transition Ævar Arnfjörð Bjarmason 2018-10-23 9:30 ` [PATCH] Poison gettext with the Ook language Johannes Schindelin 2018-10-23 10:17 ` Ævar Arnfjörð Bjarmason 2018-10-23 11:07 ` Johannes Schindelin 2018-10-23 15:00 ` Duy Nguyen 2018-10-23 16:45 ` Ævar Arnfjörð Bjarmason 2018-10-24 14:41 ` Duy Nguyen 2018-10-24 17:54 ` Ævar Arnfjörð Bjarmason 2018-10-25 3:52 ` Junio C Hamano 2018-10-25 6:20 ` Jeff King 2018-10-27 6:55 ` Junio C Hamano
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 \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ /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
email@example.com list mirror (unofficial, one of many) Archives are clonable: git clone --mirror https://public-inbox.org/git git clone --mirror http://ou63pmih66umazou.onion/git git clone --mirror http://czquwvybam4bgbro.onion/git git clone --mirror http://hjrcffqmbrq6wope.onion/git Example config snippet for mirrors Newsgroups are available over NNTP: nntp://news.public-inbox.org/inbox.comp.version-control.git nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git nntp://news.gmane.io/gmane.comp.version-control.git note: .onion URLs require Tor: https://www.torproject.org/ AGPL code for this site: git clone https://public-inbox.org/public-inbox.git