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 780220 - Distances are not using localized formats for routing
Distances are not using localized formats for routing
Status: RESOLVED FIXED
Product: gnome-maps
Classification: Applications
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: gnome-maps-maint
gnome-maps-maint
Depends on:
Blocks:
 
 
Reported: 2017-03-17 19:47 UTC by Marcus Lundblad
Modified: 2017-05-22 19:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
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 (2.40 KB, patch)
2017-03-25 15:08 UTC, goutam kumar dhandh
none 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 (2.40 KB, patch)
2017-03-25 15:10 UTC, goutam kumar dhandh
none Details | Review
Distances routing is not localized (2.40 KB, patch)
2017-03-25 15:14 UTC, goutam kumar dhandh
rejected 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 (2.40 KB, patch)
2017-03-25 18:41 UTC, goutam kumar dhandh
none 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: (1.97 KB, patch)
2017-03-25 19:01 UTC, goutam kumar dhandh
none Details | Review
Distances routing is not localized (1.97 KB, patch)
2017-03-25 19:03 UTC, goutam kumar dhandh
needs-work Details | Review
utils: Use localized numeric formats for distances (1.68 KB, patch)
2017-05-13 21:41 UTC, Marcus Lundblad
committed Details | Review

Description Marcus Lundblad 2017-03-17 19:47:05 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).
Comment 1 Marcus Lundblad 2017-03-20 08:25:04 UTC
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.
Comment 2 goutam kumar dhandh 2017-03-25 15:08:50 UTC
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
Comment 3 goutam kumar dhandh 2017-03-25 15:10:06 UTC
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
Comment 4 goutam kumar dhandh 2017-03-25 15:14:59 UTC
Created attachment 348708 [details] [review]
Distances routing is not localized
Comment 5 goutam kumar dhandh 2017-03-25 15:31:11 UTC
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
>
Comment 6 Marcus Lundblad 2017-03-25 18:28:30 UTC
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
Comment 7 goutam kumar dhandh 2017-03-25 18:32:04 UTC
ok
Comment 8 goutam kumar dhandh 2017-03-25 18:41:39 UTC
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
Comment 9 goutam kumar dhandh 2017-03-25 19:01:56 UTC
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).
Comment 10 goutam kumar dhandh 2017-03-25 19:03:30 UTC
Created attachment 348714 [details] [review]
Distances routing is not localized
Comment 11 Marcus Lundblad 2017-03-26 07:56:34 UTC
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 12 goutam kumar dhandh 2017-03-26 09:51:01 UTC
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
>
Comment 13 Marcus Lundblad 2017-03-26 11:29:42 UTC
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.
Comment 14 Marcus Lundblad 2017-05-13 21:41:32 UTC
Created attachment 351797 [details] [review]
utils: Use localized numeric formats for distances
Comment 15 Marcus Lundblad 2017-05-22 19:39:40 UTC
Attachment 351797 [details] pushed as c6b2d59 - utils: Use localized numeric formats for distances