Opened 8 years ago

Closed 8 years ago

#4414 closed (fixed)

I18N Javascript interpolate function

Reported by: tobias@… Owned by: mtredinnick
Component: Internationalization Version: master
Severity: Keywords: i18n javascript interpolate
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by mtredinnick)

why do you interpolate javascript translation strings just once? i currently have the case, that i have to replace like this:

var transObj = { numShown: numShown, numAvail: numAvail};
interpolate(gettext("%(numShown)s out of %(numAvail)s"), transObj, true));
Index: views/i18n.py
===================================================================
--- views/i18n.py       (revision 5038)
+++ views/i18n.py       (working copy)
@@ -69,9 +69,9 @@
 InterPolate = r"""
 function interpolate(fmt, obj, named) {
   if (named) {
-    return fmt.replace(/%\(\w+\)s/, function(match){return String(obj[match.slice(2,-2)])});
+    return fmt.replace(/%\(\w+\)s/g, function(match){return String(obj[match.slice(2,-2)])});
   } else {
-    return fmt.replace(/%s/, function(match){return String(obj.shift())});
+    return fmt.replace(/%s/g, function(match){return String(obj.shift())});
   }
 }
 """

Change History (5)

comment:1 Changed 8 years ago by anonymous

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Sorry for the bad ticket-formatting ;-)

why do you interpolate javascript translation strings just once? i currently have the case, that i have to replace like this:

var transObj = { numShown: numShown, numAvail: numAvail}; 
interpolate(gettext("%(numShown)s out of %(numAvail)s"), transObj, true)); 
Index: views/i18n.py
===================================================================
--- views/i18n.py       (revision 5038)
+++ views/i18n.py       (working copy)
@@ -69,9 +69,9 @@
 InterPolate = r"""
 function interpolate(fmt, obj, named) {
   if (named) {
-    return fmt.replace(/%\(\w+\)s/, function(match){return String(obj[match.slice(2,-2)])});
+    return fmt.replace(/%\(\w+\)s/g, function(match){return String(obj[match.slice(2,-2)])});
   } else {
-    return fmt.replace(/%s/, function(match){return String(obj.shift())});
+    return fmt.replace(/%s/g, function(match){return String(obj.shift())});
   }
 }
 """

comment:2 Changed 8 years ago by mtredinnick

  • Component changed from Translations to Internationalization
  • Description modified (diff)
  • Triage Stage changed from Unreviewed to Accepted

(Fixed description formatting.)

Can you explain the bug a bit more? Is it that the first couple of lines you show do not work and they should work? Or there is some other thing that should work and you have to do the first two lines of Javascript instead?

What I'm after is a clear example of something that should work, but doesn't. If that's the first two lines you showed, just point that out. Otherwise, give us an example to test with.

Just looking at the change you propose, I'm willing to believe it's just an oversight (clearly not a popular one, since nobody's reported it before that I can see). Should be easy to fix once we understand what the real issue is.

comment:3 Changed 8 years ago by tobias@…

This is not a real bug, it is more a proposal/question. In the js-function "interpolate" you just do a replacement of "%"-placeholders just once and I came to it, as i had to do two replacements like in the following string:

'%("count")s of %("amount")s messages'

If i call the actual js-interpolate-function i get the following result:

'1 of %("amount")s messages'

You can use the following lines as javascript-testcase:

var transObj = { count: 1, total: 100 }; 
var transStr = interpolate(gettext("%(count)s out of %(total)s"), transObj, true));
print(transStr);

At the moment i solved the problem like that:

var transStr = "1" + " " + gettext("out of") + " " + "100";

And i think, this isn't the elegant way.

You're right about the popularity of this bug. Maybe not many people use the way you handle javascript-translations.

comment:4 Changed 8 years ago by Simon G. <dev@…>

  • Triage Stage changed from Accepted to Ready for checkin

The explanation sounds good, and this looks like a sensible change - if you're going to replace one %(foo)s then you should replace all of them.

comment:5 Changed 8 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

(In [5849]) Fixed #4414 -- Fixed Javascript message translation to also work when there is more than one format marker in a string. Thanks, tobias@….

Note: See TracTickets for help on using tickets.
Back to Top