After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 673106 - Conversion characters are unsupported
Conversion characters are unsupported
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
3.4.x
Other Linux
: High critical
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on: 673092
Blocks:
 
 
Reported: 2012-03-29 19:27 UTC by Cosimo Cecchi
Modified: 2012-04-10 16:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
format: support %Id conversion characters in format.js (3.18 KB, patch)
2012-03-29 19:30 UTC, Cosimo Cecchi
needs-work Details | Review
format: support %Id conversion characters in format.js (3.27 KB, patch)
2012-03-29 21:25 UTC, Cosimo Cecchi
committed Details | Review
format: move shell_format_int_alternative_output() to ShellJS (3.27 KB, patch)
2012-04-10 15:24 UTC, Cosimo Cecchi
committed Details | Review

Description Cosimo Cecchi 2012-03-29 19:27:51 UTC
+++ This bug was initially created as a clone of Bug #673092 +++

The Shell has the same bug of Documents with %Id conversion specifiers, which breaks badly support for languages that use it in translations, such as Persian.

See attached patch; I committed an identical fix in Documents and it seems to work fine in my tests. I also tested it with the Shell in both en_US and fa_IR and didn't notice any problems.
Comment 1 Cosimo Cecchi 2012-03-29 19:30:19 UTC
Created attachment 210894 [details] [review]
format: support %Id conversion characters in format.js

This is needed for languages which translate numbers with an
alternative representation, such as Persian.
Comment 2 Owen Taylor 2012-03-29 20:06:50 UTC
Review of attachment 210894 [details] [review]:

Mostly looks good, I dont' think it's necessary to extend tests/misc/format.js with the difficulty of having to test under a specific locale. Just one problem I see:

::: js/misc/format.js
@@ +48,3 @@
                         case 'd':
+                            let intV = parseInt(args[i++]);
+                            if (flagsGroup == 'I')

You accept 'II' but make it different than I. probably you want to check if I is in the flags group.
Comment 3 Cosimo Cecchi 2012-03-29 21:25:19 UTC
Created attachment 210902 [details] [review]
format: support %Id conversion characters in format.js

Thanks for the review.
I think this new version of the patch should be safe enough now.
Comment 4 Owen Taylor 2012-03-29 21:28:34 UTC
Review of attachment 210902 [details] [review]:

good to commit
Comment 5 Cosimo Cecchi 2012-03-29 21:30:29 UTC
Attachment 210902 [details] pushed as 6d82aef - format: support %Id conversion characters in format.js
Comment 6 Jasper St. Pierre (not reading bugmail) 2012-04-10 04:58:29 UTC
This broke the extension prefs tool -- we can import Shell (but we shouldn't), but we can't import Meta, which Shell depends on, so it breaks.

I don't know what to do here -- I don't want to add a Printf.typelib or whatever, and I don't want to make gnome-shell-extension-prefs depend on Shell and Meta.
Comment 7 Giovanni Campagna 2012-04-10 11:44:09 UTC
What about moving the utility function to ShellJS?
Not the best place, agreed, but it can be shared between the prefs-tool and the shell.
Comment 8 Cosimo Cecchi 2012-04-10 14:24:38 UTC
ShellJS looks like a good compromise to me...otherwise another quick fix could be just importing another copy of format.js (not depending on Shell) for the extension prefs tool, and use that (since it doesn't use %d in the interface).
I think the best solution long-term would be moving the format functions in GJS itself, since pretty much any module that wants to have translatable strings in the user interface needs it.
Comment 9 Owen Taylor 2012-04-10 15:14:37 UTC
ShellJS sounds right to me - I definitely don't want to have multiple copies of format.js that have different capabilities and have translators be able to use %Id for one part of the code and not for another.
Comment 10 Cosimo Cecchi 2012-04-10 15:24:33 UTC
Created attachment 211760 [details] [review]
format: move shell_format_int_alternative_output() to ShellJS

Okay, here's a patch that does that.
Comment 11 Owen Taylor 2012-04-10 15:38:03 UTC
Review of attachment 211760 [details] [review]:

Looks good to me
Comment 12 Cosimo Cecchi 2012-04-10 16:52:56 UTC
Attachment 211760 [details] pushed as 49d8e6d - format: move shell_format_int_alternative_output() to ShellJS

Thanks, pushed.