public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Maximiliano Sandoval <m.sandoval@proxmox.com>
To: "Kefu Chai" <k.chai@proxmox.com>
Cc: pve-devel@lists.proxmox.com
Subject: Re: [PATCH proxmox-i18n v2] add pgettext() and npgettext() support for context-aware translations
Date: Mon, 02 Mar 2026 09:15:28 +0100	[thread overview]
Message-ID: <s8ojyvuk5pb.fsf@toolbox> (raw)
In-Reply-To: <DGQAMFGI228C.2B28HGE88ODBC@proxmox.com> (Kefu Chai's message of "Sat, 28 Feb 2026 12:00:12 +0800")

"Kefu Chai" <k.chai@proxmox.com> writes:

> On Tue Feb 17, 2026 at 11:45 PM CST, Maximiliano Sandoval wrote:
>
> Hi Maximiliano,
>
> Thanks for the detailed testing and feedback! I've addressed all the
> points you raised.
>
>> Kefu Chai <k.chai@proxmox.com> writes:
>>
>> Thanks for the update. Comments below.
>>
>>> This commit adds message context (msgctxt) support to the JavaScript
>>> [..]
>>>  Makefile |  1 +
>>>  po2js.pl | 69 ++++++++++++++++++++++++++++++++++++++++++++++++--------
>>>  2 files changed, 61 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 86bd723..3feaee7 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -155,6 +155,7 @@ define potupdate
>>>        --package-version="$(shell cd $(2);git rev-parse HEAD)" \
>>>        --msgid-bugs-address="<support@proxmox.com>" \
>>>        --copyright-holder="Copyright (C) Proxmox Server Solutions GmbH <support@proxmox.com> & the translation contributors." \
>>> +      --keyword=npgettext:1c,2,3 \
>>
>> You were right in your previous reply, however as I mentioned, gettext
>> by default has keys for gettext, ngettext, pgettext and npgettext. There
>> is no need to add this here.
>
> I investigated this further and found that while this is true for C/C++, it's
> not true for JavaScript. The --keyword flag is actually required.
>
> According to the GNU gettext manual [1], the default keywords are
> language-specific:
>
>   For C, C++, GCC-source:
>     gettext, dgettext:2, dcgettext:2, ngettext:1,2, dngettext:2,3,
>     dcngettext:2,3, gettext_noop, pgettext:1c,2, dpgettext:2c,3,
>     dcpgettext:2c,3, npgettext:1c,2,3, dnpgettext:2c,3,4, dcnpgettext:2c,3,4
>
>   For JavaScript, TypeScript, TSX:
>     _, gettext, dgettext:2, dcgettext:2, ngettext:1,2, dngettext:2,3,
>     pgettext:1c,2, dpgettext:2c,3
>
> Notice that npgettext is present in C/C++ defaults but ABSENT from JavaScript.
>
> The xgettext source code confirms this. In gettext-tools/src/x-c.c [2]:
>
>   static void init_keywords () {
>     if (default_keywords) {
>       x_c_keyword ("gettext");
>       x_c_keyword ("dgettext:2");
>       x_c_keyword ("dcgettext:2");
>       x_c_keyword ("ngettext:1,2");
>       x_c_keyword ("dngettext:2,3");
>       x_c_keyword ("dcngettext:2,3");
>       x_c_keyword ("gettext_noop");
>       x_c_keyword ("pgettext:1c,2");
>       x_c_keyword ("dpgettext:2c,3");
>       x_c_keyword ("dcpgettext:2c,3");
>       x_c_keyword ("npgettext:1c,2,3");      /* <- present for C */
>       x_c_keyword ("dnpgettext:2c,3,4");
>       x_c_keyword ("dcnpgettext:2c,3,4");
>       // ...
>     }
>   }
>
> But in gettext-tools/src/x-javascript.c [3]:
>
>   static void init_keywords () {
>     if (default_keywords) {
>       x_javascript_keyword ("gettext");
>       x_javascript_keyword ("dgettext:2");
>       x_javascript_keyword ("dcgettext:2");
>       x_javascript_keyword ("ngettext:1,2");
>       x_javascript_keyword ("dngettext:2,3");
>       x_javascript_keyword ("pgettext:1c,2");
>       x_javascript_keyword ("dpgettext:2c,3");
>       x_javascript_keyword ("_");
>       /* npgettext is NOT here */      /* <- absent for JavaScript */
>       default_keywords = false;
>     }
>   }
>
> The official release notes for gettext 0.18.3 (July 2013) [4] explain
> the reason of the difference:
>
>
>   "JavaScript:
>    xgettext now partially supports JavaScript. Since the current
>    JavaScript specification (ECMA-262) does not define the standard
>    set of formatting methods nor translation functions, the
>    implementation supports only a limited set of formatting methods
>    and translation functions commonly used in Gjs and other popular
>    JavaScript implementations and libraries."
>
> This was a deliberate design decision to include only commonly-used
> functions in the JavaScript defaults. The context+plural combination
> (npgettext) was apparently not common enough in 2013 to be included.
>
> We can confirm with testing:
>
>   # C: npgettext extracted by default
>   $ echo 'npgettext("ctx", "s", "p", n);' > test.c
>   $ xgettext test.c -o - | grep msgctxt
>   msgctxt "ctx"  # GOOD!
>
>   # JavaScript: npgettext NOT extracted without --keyword
>   $ echo 'npgettext("ctx", "s", "p", n);' > test.js
>   $ xgettext --language=JavaScript test.js -o - | grep msgctxt
>   (empty)  # MISSING!
>
>   # JavaScript WITH --keyword flag
>   $ xgettext --language=JavaScript --keyword=npgettext:1c,2,3 test.js -o - | grep msgctxt
>   msgctxt "ctx"  # GOOD!
>
> References:
>
> [1] GNU gettext manual - xgettext Invocation (Language-specific default keywords)
>     https://www.gnu.org/software/gettext/manual/html_node/xgettext-Invocation.html
>
> [2] gettext source: x-c.c (C language keyword definitions)
>     https://github.com/autotools-mirror/gettext/blob/master/gettext-tools/src/x-c.c
>
> [3] gettext source: x-javascript.c (JavaScript language keyword definitions)
>     https://github.com/autotools-mirror/gettext/blob/master/gettext-tools/src/x-javascript.c
>
> [4] gettext 0.18.3 Release Notes (July 2013 - JavaScript support added)
>     https://git.savannah.gnu.org/gitweb/?p=gettext.git;a=blob;f=NEWS
>     Or: https://github.com/autotools-mirror/gettext/blob/master/NEWS

Thanks for digging up all this! When you send a new version, please link
the GNU gettext manual in the commit message with a brief explanation of
why it is needed.

>>
>>>        --output="$(1)".pot
>>>  endef
>>>  
>>> diff --git a/po2js.pl b/po2js.pl
>>> index 316c0bd..4b7b044 100755
>>> [..]
>>>  
>>>  if ($outfile) {
>>
>>
>> I tested this with pve-manager.
>>
>> - Sprinkled a few uses of pgettext
>> - Extracted the strings with make do_update at proxmox-i18n
>> - Translated two strings with context
>> - Checked that the translated string were visible at the web UI and that
>>   each one had the correct translation given its context
>>
>> To make the above work I had to patch proxmox-biome to accept the
>> ngettext and pngettext keywords:
>>
>> modified   src/biome.json
>> @@ -128,7 +128,9 @@
>>              "Proxmox",
>>              "eslint",
>>              "ngettext",
>> -            "gettext"
>> +            "gettext",
>> +            "pgettext",
>> +            "npgettext"
>>          ]
>>      }
>>  }
>
> Updated proxmox-biome/src/biome.json to add pgettext and npgettext
> to the globals list. Will send a separate patch to the mailing list.
>
>>
>> Otherwise `make deb` would fail when building the .debs.
>>
>> I also had to modify the index.html.tpl to pve-manager to have default
>> implementations for these fns, otherwise the UI won't work if there is
>> no language selected (which is the default):
>
> Done. Added default implementations to all three locations:
>   - pve-manager/www/index.html.tpl
>   - proxmox-backup/www/index.hbs
>   - pmg-gui/pmg-index.html.tt
>
> Will update them separately, and bump up the submodules in the next
> revision of this patch once these patches land.
>
> FWIW, to replicate your testing approach, I performed the following
> verification:
>
> 1. Added pgettext/npgettext test translation to zh_CN.po, 
> 2. Generated JavaScript with po2js.pl:
>    ./po2js.pl -t pve -v "3.6.6" -o pve-lang-zh_CN-test.js zh_CN.po
> 3. Verified context-aware translations in generated JavaScript manually.
>
> Also, I built all .deb packages with:
>
>   make deb
>
>
>>
>> modified   www/index.html.tpl
>> @@ -27,6 +27,8 @@
>>      <script type='text/javascript'>
>>          function gettext(message) { return message; }
>>          function ngettext(singular, plural, count) { return count === 1 ? singular : plural; }
>> +        function pgettext(context, message) { return message; }
>> +        function pngettext(context, singular, plural, count) { return count === 1 ? singular : plural; }
>>      </script>
>>      [% END %]
>>      [%- IF debug %]
>>
>> There are similar files at pmg-gui and proxmox-backup-server that would
>> also need updating.

-- 
Maximiliano




      reply	other threads:[~2026-03-02  8:14 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-05 10:16 [PATCH proxmox-i18n v1] " Kefu Chai
2026-02-05 13:15 ` Maximiliano Sandoval
2026-02-06  5:44   ` Kefu Chai
2026-02-06  5:47 ` [PATCH proxmox-i18n v2] " Kefu Chai
2026-02-17 15:45   ` Maximiliano Sandoval
2026-02-28  4:00     ` Kefu Chai
2026-03-02  8:15       ` Maximiliano Sandoval [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

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

  git send-email \
    --in-reply-to=s8ojyvuk5pb.fsf@toolbox \
    --to=m.sandoval@proxmox.com \
    --cc=k.chai@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal