Also: try typing in type in

1+(5+6)

This shows as:

1,+(5,+6)

Initial problem confirmed for arithmetic operator precedence mode.
Works find in left-to-right precedence mode.
Adjusting summary accordingly.

> Also: try typing in type in
> 1+(5+6)
> This shows as:
> 1,+(5,+6)

I'm not seeing this problem (in either AOP or LTR modes)
for the English locale I'm running in.

What is the English locale you are running in?

chris@ubuntu:~$ locale
LANG=en_AU.UTF-8
LC_CTYPE="en_AU.UTF-8"
LC_NUMERIC="en_AU.UTF-8"
LC_TIME="en_AU.UTF-8"
LC_COLLATE="en_AU.UTF-8"
LC_MONETARY="en_AU.UTF-8"
LC_MESSAGES="en_AU.UTF-8"
LC_PAPER="en_AU.UTF-8"
LC_NAME="en_AU.UTF-8"
LC_ADDRESS="en_AU.UTF-8"
LC_TELEPHONE="en_AU.UTF-8"
LC_MEASUREMENT="en_AU.UTF-8"
LC_IDENTIFICATION="en_AU.UTF-8"
LC_ALL=

Here's mine:

$ locale
LANG=en_US.UTF-8
LC_CTYPE="en_US.UTF-8"
LC_NUMERIC="en_US.UTF-8"
LC_TIME="en_US.UTF-8"
LC_COLLATE="en_US.UTF-8"
LC_MONETARY="en_US.UTF-8"
LC_MESSAGES="en_US.UTF-8"
LC_PAPER="en_US.UTF-8"
LC_NAME="en_US.UTF-8"
LC_ADDRESS="en_US.UTF-8"
LC_TELEPHONE="en_US.UTF-8"
LC_MEASUREMENT="en_US.UTF-8"
LC_IDENTIFICATION="en_US.UTF-8"
LC_ALL=
$

I suspect that the problem with gcalctool is because refresh_display() calls ui_set_display(). However, ui_set_display calls:

        if (v->noparens == 0) {
            localize_number(localized, str);
            str = localized;
        }

Basically, it looks like it's trying to localize the whole expression, but this function is only build to localize an individual *number*. Thus is adds in the comma every 3rd character regardles... 

Not sure how you'd get around this, but I think that's the problem here. 

P.S. sorry in advance if I'm on the wrong track. I ran gcalctool through a debugger and that's what I saw happening. 

Any and all help is appreciated Chris. Thanks!

No probs. 

If I'm on the right track, I've two thoughts on the matter. One way to deal with this would be to create a new function called localize_expression() that parses the expression for numbers and on each number calls localize_number(). 

Alternatively, the problem portion of the code seems to be run only when you type in a new number to the calculator, so basically just loop backwards in *str until you find the start of the string, copy this number into a new string and call localize_number() on this. Then whack the number back into *str. 

Sorry, it's late here. What I meant to say was:

"loop backwards in *str until you find the start of the *number*"

Just more fun on this:

when I run the calculator with locale en_US.UTF-8 and type 0.001 and then click the 1/x button "1,/(0.001)" will be displayed, hitting "=" displays "1,,000". When I change my locale to "de_DE" (with this locale "," is the decimal separator, "." the thousand separator) and type "0,00" or "0.00" the displayed text gets converted to 0,,00.

Version: 5.22.0 from Debian testing.

Created attachment 109104 [details] [review]
patch to fix the thousand separator problem

the change is in gactool/display.c

I am not sure where to submit the patch as I found the bug through ubuntu launchpad, and there is one duplicate. I submitted to either.

---

The issue was with the localize_number() function. It assumed that the string to format does not contain ,+*,etc. So it gave incorrect result when the expression already includes thousand separators. Moreover, the logic was not working well when there were non-digit characters in the expression e.g. (,),+,%, etc.

Minor update to the localize_number() function in gcalctool/display.c, provided in the attached patch. Basically we want to identify 3 contiguous digits, and we need to add the separator only if the next char is a number. If a non-digit char occurs, I reset the digit counter.

I tested this patch in all calculator modes satisfactorily. Things like 3,333,333,336+36,985 are working now.

I would appreciate if someone reviews this to see if it is acceptable.

I've merged all the thousands separator bugs into bug 527669 with a recent patch that appears to have fixed most of these problems. Thanks if you took the time to make a patch, I reviewed them but unfortunately none of them fixed all the problems simultaneously. Please test the patch and raise any issues in the new bug. Thank you.

*** This bug has been marked as a duplicate of 527669 ***