GNOME Bugzilla – Bug 780220
Distances are not using localized formats for routing
Last modified: 2017-05-22 19:39:44 UTC
Distances in the routing results are not properly localized. For example in a Swedish (and most other European) locale, the decimal separator is the comma. But in the routing results it shows with a period. The code (in utils.js) does "%f km".format(distance) (and similar for sub-km distances and imperial units). It probably needs to use some locale-aware equivalent (glib or JS standard library functionallity, I suppose).
After some useful hint from ptomato on IRC, we should use .toLocaleString() instead of the format() prototype, as that one doesn't handle localization. As this would involve string change it will have to be done for 3.26.
Created attachment 348706 [details] [review] Distances in the routing results are not properly localized. For example in a Swedish (and most other European) locale, the decimal separator is the comma. But in the routing results it shows with a period.The code (in utils.js) does "%f km".format(distan
Created attachment 348707 [details] [review] Distances in the routing results are not properly localized. For example in a Swedish (and most other European) locale, the decimal separator is the comma. But in the routing results it shows with a period.The code (in utils.js) does "%f km".format(distan
Created attachment 348708 [details] [review] Distances routing is not localized
Comment on attachment 348708 [details] [review] Distances routing is not localized >From 19d0a4eb4eca2099082a8209f086c004ae327881 Mon Sep 17 00:00:00 2001 >From: Goutam Kumar Dhandh <gdhandh@gmail.com> >Date: Sat, 25 Mar 2017 20:07:11 +0530 >Subject: [PATCH] Distances in the routing results are not properly localized. > For example in a Swedish (and most other European) locale, the decimal > separator is the comma. But in the routing results it shows with a period.The > code (in utils.js) does "%f km".format(distance) (and similar for sub-km > distances and imperial units). > >https://bugzilla.gnome.org/show_bug.cgi?id=780220 >--- > src/utils.js | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > mode change 100644 => 100755 src/utils.js > >diff --git a/src/utils.js b/src/utils.js >old mode 100644 >new mode 100755 >index 1e29dbe..deba978 >--- a/src/utils.js >+++ b/src/utils.js >@@ -331,10 +331,9 @@ function prettyTime(time) { > seconds = seconds % 60; > let hours = Math.floor(minutes / 60); > minutes = minutes % 60; >- > let labelledTime = ""; > if (hours > 0) >- labelledTime += _("%f h").format(hours)+' '; >+ labelledTime += _("%f h").format(hours)+' '; > if (minutes > 0) > labelledTime += _("%f min").format(minutes); > if (hours === 0 && minutes === 0) >@@ -349,10 +348,10 @@ function prettyDistance(distance, noRound) { > if (distance >= 1000 && !noRound) { > distance = Math.round(distance / 1000 * 10) / 10; > /* Translators: This is a distance measured in kilometers */ >- return _("%f km").format(distance); >+ return _("%f km").toLocalizedString(distance); > } else > /* Translators: This is a distance measured in meters */ >- return _("%f m").format(distance); >+ return _("%f m").toLocalizedString(distance); > } else { > // Convert to feet > distance = Math.round(distance * 3.2808399); >@@ -360,10 +359,10 @@ function prettyDistance(distance, noRound) { > // Convert to miles when distance is more than 0.2 mi > distance = Math.round(distance / 5280 * 10) / 10; > /* Translators: This is a distance measured in miles */ >- return _("%f mi").format(distance); >+ return _("%f mi").toLocalizedString(distance); > } else > /* Translators: This is a distance measured in feet */ >- return _("%f ft").format(distance); >+ return _("%f ft").toLocalizedString(distance); > } > } > >-- >2.9.3 >
Review of attachment 348708 [details] [review]: This patch is actually broken as is right now. The commit message format needs some changes. See https://wiki.gnome.org/Git/CommitMessages for example. In this case, the module would be "utils" And I think the message body might be a bit too detailed with the complete bug description. Maybe something in this case the first line would suffice, something like: utils: Use localized format when formatting distance strings ::: src/utils.js @@ +334,3 @@ let labelledTime = ""; if (hours > 0) + labelledTime += _("%f h").for(hours)+' '; This seems like a mistake, this shouldn't have been changed I think. @@ +349,3 @@ distance = Math.round(distance / 1000 * 10) / 10; /* Translators: This is a distance measured in kilometers */ + return _("%f km").toLocalizedString(distance); This is unfortunatly wrong and doesn't actually work :( It should be something like: _("%s km").format(distance.toLocaleString()) @@ +352,3 @@ } else /* Translators: This is a distance measured in meters */ + return _("%f m").toLocalizedString(distance); Same as above @@ +360,3 @@ distance = Math.round(distance / 5280 * 10) / 10; /* Translators: This is a distance measured in miles */ + return _("%f mi").toLocalizedString(distance); Same as above @@ +363,3 @@ } else /* Translators: This is a distance measured in feet */ + return _("%f ft").toLocalizedString(distance); Same as above
ok
Created attachment 348710 [details] [review] Distances in the routing results are not properly localized. For example in a Swedish (and most other European) locale, the decimal separator is the comma. But in the routing results it shows with a period.The code (in utils.js) does "%f km".format(distan
Created attachment 348713 [details] [review] From: Goutam Kumar Dhandh<gdhandh@gmail.com> Date:march/26/2017 Subject: [PATCH]Distances routing is not localized This commit address the bug: https://bugzilla.gnome.org/show_bug.cgi?id=780220 routing results it shows with a period.The code (in utils.js) does "%f km".format(distance) (and similar for sub-km distances and imperial units).
Created attachment 348714 [details] [review] Distances routing is not localized
Review of attachment 348714 [details] [review]: I'd still like the commit message follow the established GNOME conventions. ::: src/utils.js @@ +349,3 @@ distance = Math.round(distance / 1000 * 10) / 10; /* Translators: This is a distance measured in kilometers */ + return _("%f km").format(distance.toLocaleString()); The format string should use %s instead of %f, as the patch is right now, it's actually broken. The .format() function will interpret the argument value as a floating point value (since the format string uses %f), so if this is run in an English locale, distance.toLocaleString() would return a string such as "1.1". This is coearced into a float when interpreted by .format() and it will be formatted into the resulting string correctly. However, if you run this with a "sv_SE" locale (for example), the distance.toLocaleString() call in this example would have returned the string "1,1". When this is handled by .format() for parsing a float argument as it seems the trailing characters ",1" is ignored and the value is printed as "1". Thus this actually has the effect of basically truncating the decimal part when the decimal separator is not ".". So, therefore the correct way is to use %s in the format string. @@ +352,3 @@ } else /* Translators: This is a distance measured in meters */ + return _("%f m").format(distance.toLocaleString()); Same as above @@ +360,3 @@ distance = Math.round(distance / 5280 * 10) / 10; /* Translators: This is a distance measured in miles */ + return _("%f mi").format(distance.toLocaleString()); Same as above @@ +363,3 @@ } else /* Translators: This is a distance measured in feet */ + return _("%f ft").format(distance.toLocaleString()); Same as above
Comment on attachment 348714 [details] [review] Distances routing is not localized >From 9ddf1742ba99523059eb891f8d61c483302a2389 Mon Sep 17 00:00:00 2001 >From: Goutam Kumar Dhandh <gdhandh@gmail.com> >Date: Sun, 26 Mar 2017 00:25:48 +0530 >Subject: [PATCH] From: Goutam Kumar Dhandh<gdhandh@gmail.com> > Date:march/26/2017 Subject: [PATCH]Distances routing is not localized This > commit address the bug: > >https://bugzilla.gnome.org/show_bug.cgi?id=780220 > >routing results it shows with a period.The > code (in utils.js) does "%f km".format(distance) (and similar for sub-km > distances and imperial units). >--- > src/utils.js | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > >diff --git a/src/utils.js b/src/utils.js >index deba978..f0e7335 100755 >--- a/src/utils.js >+++ b/src/utils.js >@@ -348,10 +348,10 @@ function prettyDistance(distance, noRound) { > if (distance >= 1000 && !noRound) { > distance = Math.round(distance / 1000 * 10) / 10; > /* Translators: This is a distance measured in kilometers */ >- return _("%f km").toLocalizedString(distance); >+ return _("%s km").format(distance.toLocaleString()); > } else > /* Translators: This is a distance measured in meters */ >- return _("%f m").toLocalizedString(distance); >+ return _("%s m").format(distance.toLocaleString()); > } else { > // Convert to feet > distance = Math.round(distance * 3.2808399); >@@ -359,10 +359,10 @@ function prettyDistance(distance, noRound) { > // Convert to miles when distance is more than 0.2 mi > distance = Math.round(distance / 5280 * 10) / 10; > /* Translators: This is a distance measured in miles */ >- return _("%f mi").toLocalizedString(distance); >+ return _("%s mi").format(distance.toLocaleString()); > } else > /* Translators: This is a distance measured in feet */ >- return _("%f ft").toLocalizedString(distance); >+ return _("%s ft").format(distance.toLocaleString()); > } > } > >-- >2.9.3 >
Review of attachment 348714 [details] [review]: It looks as if this patch is made as a diff from a previous patch. The patch should be made against the HEAD of master. So if you do subsequent patches and have commited in git locally, you should use git commit --amend to rewrite the changes so that it becomes one "stand-alone" commit.
Created attachment 351797 [details] [review] utils: Use localized numeric formats for distances
Attachment 351797 [details] pushed as c6b2d59 - utils: Use localized numeric formats for distances