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 632658 - Thousands separators cannot be parsed by our CLI tools
Thousands separators cannot be parsed by our CLI tools
Status: RESOLVED FIXED
Product: gnome-calculator
Classification: Core
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: gcalctool maintainers
gcalctool maintainers
Depends on: 589569
Blocks:
 
 
Reported: 2010-10-20 09:16 UTC by Robin Sonefors (ozamosi)
Modified: 2012-12-11 02:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Ignore thousands separator in CLI tools. (2.87 KB, patch)
2012-10-09 21:09 UTC, PioneerAxon
needs-work Details | Review
Ignore thousands separator in CLI tools. (1.80 KB, patch)
2012-10-14 23:30 UTC, PioneerAxon
needs-work Details | Review
Ignore thousands separator in CLI tools. (1.77 KB, patch)
2012-10-15 20:55 UTC, PioneerAxon
needs-work Details | Review
Ignore thousands separator in CLI tools. (1.55 KB, patch)
2012-12-05 21:42 UTC, PioneerAxon
committed Details | Review

Description Robin Sonefors (ozamosi) 2010-10-20 09:16:47 UTC
...unless the locale's thousands separator is " ".

I fixed this in 5.91.0, but it was reverted again in df984fc20501db04fc94fba1f30f48fd19d95da3

For instance, this should not return parser error:
LC_NUMERIC=en_US gcalctool -s "99,999.00"
Comment 1 Robert Ancell 2010-10-20 09:33:41 UTC
For this to work the parser needs to be aware of this (In some locales the separator is '.').  I've failed to do it and the comments you added indicated you had much the same problem!  The solution is to improve the parser, see bug 589569.
Comment 2 PioneerAxon 2012-10-09 21:09:15 UTC
Created attachment 226144 [details] [review]
Ignore thousands separator in CLI tools.
Comment 3 Robert Ancell 2012-10-14 07:12:53 UTC
Review of attachment 226144 [details] [review]:

Can you update this for the vala port? Thanks.
Comment 4 PioneerAxon 2012-10-14 23:30:33 UTC
Created attachment 226432 [details] [review]
Ignore thousands separator in CLI tools.
Comment 5 Robert Ancell 2012-10-15 03:21:04 UTC
Review of attachment 226432 [details] [review]:

::: src/equation.vala
@@ -108,3 @@
     public int wordlen;
     public AngleUnit angle_units;
-    private string expression;

There is no need to make this public, you should modify the string before creating the object.

::: src/gcalccmd.vala
@@ +24,3 @@
+    if (tsep_string == null || tsep_string == "")
+        tsep_string = " ";
+    e.expression = string.joinv ("", e.expression.split(tsep_string, 0));

Use string.replace instead - it will be simpler to read
Comment 6 PioneerAxon 2012-10-15 20:55:20 UTC
Created attachment 226504 [details] [review]
Ignore thousands separator in CLI tools.

Uses string.replace () method.
Comment 7 Robert Ancell 2012-11-08 02:17:00 UTC
Review of attachment 226504 [details] [review]:

::: src/equation.vala
@@ -108,3 @@
     public int wordlen;
     public AngleUnit angle_units;
-    private string expression;

Same as last time. Keep this private and modify the expression before creating an Equation object.
Comment 8 PioneerAxon 2012-12-05 21:42:45 UTC
Created attachment 230832 [details] [review]
Ignore thousands separator in CLI tools.
Comment 9 Robert Ancell 2012-12-11 02:26:54 UTC
Review of attachment 230832 [details] [review]:

Thanks Arth!
Comment 10 Robert Ancell 2012-12-11 02:28:04 UTC
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.