Hi. Which version of gcalctool is this?
Help->About will give you this information.
Also, what locale are you running in? Thanks.

Hi.  The about box says Gcalctool 4.3.44.
I'm running Gnome 2.5.5 on Fedora Core 1.
LANG=en_US.UTF-8, none of LC_* are set.

Adding Miloslav to the cc: list. 

Miloslav, I need a favour here please. I'm running in 
the C locale here, where there is no thousands separator, 
and therefore I'm not seeing a problem. Could you see if
you reproduce the problem and possibly suggest a fix?

Thanks.
 

Yes, I can reproduce it, but finding the fix won't be that easy.
Unfortunately add_tsep () and remove_tsep () [via MPstr_to_num ()]
are sprinkled throughout the code and I can't find any obvious
rules for that.
(the bug is that add_tsep () is called twice without remove_tsep ()).

What really needs to be done is
* core code computes always in ASCII, '.'
* adding thousands separators, conversion to locale digits, ...
  happens only immediately before displaying without modifying the
  "master" copy.

Last time I said I'll let somebody other do it, but it looks
it is just my fate :)

I'll try to prepare a patch in a few days.

Created attachment 24802 [details] [review]
Remove all locale processing out of the core code

I can't reproduce the original bug with the attached patch.
It implements the approach suggested above, no core code
uses the locale values (can be confirmed by grepping for
'\<\(tsep\|radix\)\>'). The locale is taken into account
only immediately before displaying the text.

Thanks Miloslav!  This is a really nice fix.
*Much* cleaner then my previous code. I've checked
it into CVS HEAD, and just released v4.3.48
which should be turning up under:

  http://ftp.gnome.org/pub/GNOME/sources/gcalctool/4.3/

very soon. I'm going to close the bug. Paisa,
could you try out the new version and make sure that 
everything works fine for you to please? If it doesn't 
then just reopen it.

I'm also going to send a message to desktop-devel to 
get others to give this a workout as it's an important 
change and I want to make sure that it's all working 
perfectly by GNOME 2.6 GA. 

It's much better now.  There's still a small glitch though.

1) View->show thousand seperator.
2) type in a 6-digit number
3) hit "+/-"

There's an extraneous comma at the front (e.g. -,123,456)

Yes, I have noticed that too; I didn't want to merge the patch
with this (and other possible enhancements, like using
localeconv()->grouping).
Attaching a patch, now confident this won't break anything else.

Created attachment 24853 [details] [review]
Don't add a thousands separator before the minus sign

Thanks Miloslav. I've checked this change into
CVS HEAD and bumped the version number to 4.3.49
in configure.in. I haven't generated a new tarball
as it's a simple adjustment to make to the previous 
version. Paisa, could you give it a try and see if
it totally fixes this problem please? I'll keep the
bug open until you've had a chance to verify it.

Thanks.

Thanks.  The extraneous comma is gone.  While testing it, I found
another minor bug.  It's also a display bug so I tag it along in this
report.

  punch in  . 25 + 2 =

I got 2.25 fine.  But while I was typeing ".25", the display was blank.

Miloslav, the problem is in the code at the beginning of
set_display() in gtk.c (about line 1863):

    if (str == NULL || *str == '.')
	str = " ";
    else
      {
	localize_number(localized, str);
	    str = localized;
      }

I replaced this with just:

    localize_number(localized, str);
    str = localized;

and this fixed this particular problem, but I'm not sure
exactly what you are trying to achieve with this if test,
so I'll leave it to you to suggest a real fix. Thanks.

Sorry, that is my thinko.
The code is replacing the original 
    str != NULL && strlen(str) != 0 ? str : " ",
a few lines below, so the condition should be
if (str == NULL || *str == 0)

Created attachment 24872 [details]
Fix for the disappearing ".25" problem.

Thanks again Miloslav. I've added an attachment
to make sure that I understood your last comment
currently. I took the liberty of adjusting the
code snippet to the OneTrueStyle(TM) too ;-)
I've commited to CVS HEAD and bumped the version
number to 4.3.50 in configure.in (no new tarball
though). Paisa, could you try out this patch and 
let us know if it works on not please?

Thanks.

I compiled from HEAD minutes ago.  Work fine now.  You guys are quick :-).

It's more a case of there is limited time left to 
do anything for GNOME 2.5/6 ;-)  Thanks!