Digit Grouping wrong after the decimal point

[Daniel Serodio]

Originally reported on Google Code with ID 478

Steps to reproduce:
1) Enable Digit Grouping
2) Calculate 1/3

Expected behavior: The output is 0,33 333 333 333 333 333 333

Experienced behavior: The output should be 0,333 333 333 333 333 333 33

Product version: SpeedCrunch 0.11 (Qt 4.8.5)

Operating system: Mac OS X 10.9.1

Additional Information:

Reported by dserodio on 2014-02-11 15:41:28

[Former user]

I can't reproduce this on Ubuntu. Are you able to test on Windows or Linux?

Reported by helder.pereira.correia on 2014-02-12 05:15:55

[Former user]

I can reproduce it on Windows, if I set the radix character to "Comma" (it's a dot by
default on my system). The digit grouping algorithm detects the fractional part by
looking for a dot only (line 227 of syntaxhighlighter.cpp), so it does not work with
commas. I don't know if it should detect it using both characters unconditionally,
or only using the current radix character from the settings.

Reported by [e-mail] on 2014-02-24 19:23:44

[Former user]

@Tey: Ah, yes, I see it now. I think it should consider comma and dot unconditionally.
Can you prepare a patch?

Reported by helder.pereira.correia on 2014-02-24 21:57:46 - Status changed: Accepted - Labels added: Type-Defect, Priority-High, OpSys-All, Milestone-0.12

[Former user]

The patch is pretty straightforward, but ok, I'll write it ASAP.

Reported by [e-mail] on 2014-02-25 17:59:09

[Former user]

Fixed in this commit:
https://github.com/Tey/SpeedCrunch/commit/689e787e8ae591f6058e3c633075cc5b04c04bee

Dunno if you can apply directly though, as its parent is not the official master.

Reported by [e-mail] on 2014-05-01 15:52:27

[Former user]

I'm confused. Trying with master (7a34226) and it works fine with comma, but not with
dot. When I apply your patch, neither character works. This is Ubuntu 13.10 with Qt
4.8.4.

Reported by helder.pereira.correia on 2014-05-06 06:58:27

[Former user]

This is strange, because master and patch work fine for me on Ubuntu 12.04 with Qt 4.8.1.

Here's how I tested it:
 mv ~/.config/SpeedCrunch ~/.config/SpeedCrunch.bak
 git clone https://github.com/speedcrunch/SpeedCrunch.git
 cd SpeedCrunch/src/
 qmake . && make
 ./speedcrunch
Digit grouping works with dots, but not with commas (provided digit grouping is enabled
in the options).
Now I apply the patch:
 wget -O - 'https://github.com/Tey/SpeedCrunch/commit/689e787e8ae591f6058e3c633075cc5b04c04bee.patch'
| git apply
 make
 ./speedcrunch
Digit grouping works with both now.

Can you try using these commands in order to check the problem does not come from your
config file or a tainted working copy ?

I doubt the problem comes from the Qt versions difference, as digit grouping only uses
basic functionalities from Qt.

Reported by [e-mail] on 2014-05-06 13:24:27

[Former user]

Tried the instructions and I got the very same results.

Reported by helder.pereira.correia on 2014-05-07 07:21:30

[Former user]

Let me rephrase that just to be on the safe side: I got the very same results as I got
before. So I'm still confused :)

Reported by helder.pereira.correia on 2014-05-07 07:22:33

[Former user]

I'll try reproducing this behavior using the same Ubuntu and Qt versions.

In the meantime, can you try compiling the latest stable (commit 9307757 I guess),
and see if the same behavior happens please ? This is to find out whether the problem
comes from the Qt version or an in-between commit.

Reported by [e-mail] on 2014-05-07 11:48:42

[Former user]

No difference when testing against past commits. Without the patch comma works, dot
doesn't. With the patch none works.

Reported by helder.pereira.correia on 2014-05-08 07:27:06

[Former user]

Thanks for the feedbacks. I tried with an up-to-date Ubuntu 13.10 an Qt 4.8.4, but failed
at reproducing your behavior. When you say it works with comma without the patch, I
guess it doesn't really work properly, right ? I mean, "12345,12345" is formatted as
"12 345,12 345" instead of "12 345,123 45".

If I understand correctly, digit grouping after the decimal point used to work for
you, right ? If so, do you remember having changed anything to your system since that
time ? I was about to ask you to check that your Qt dev package is up to date, but
the latest update for Ubuntu 13.10 is from last December, so I doubt this is the problem.

Also, can you tell me how is the expression formatted when you enter "12345.12345",
"12345,12345" and "12345.12345 + 12345.12345", without the patch please ?

Reported by [e-mail] on 2014-05-08 16:44:42

[Former user]

12 345.123 45
= 12 345,12 345

12 345,12 345
= 12 345,12 345

12 345.123 45 + 12 345.123 45
= 24 690,2 469

12 345.123 45
= 12 345.123 45

12 345,12 345
= 12 345.123 45

12 345.123 45 + 12 345.123 45
= 24 690.246 9

Reported by helder.pereira.correia on 2014-05-08 19:38:27

[Former user]

Errr ... it works then ? The expected behavior is 12345.12345 => "12 345.123 45", as
digits are grouped by 10e3 groups before the decimal point, and 10e-3 after the decimal
point.

Reported by [e-mail] on 2014-05-08 20:47:36

[Former user]

If that's how it should work, then yes, it's working. But I got the exact opposite impression
reading the original post with regard to expected vs. experienced. It's now clear that
you and the reporter do not have the same opinion. And you actually found the comma
bug which created the confusion and looked like you were talking about the same thing
:) Bottom line to me at this point: this is not a valid bug report, but there was actually
a bug and the patch fixes it. Is this correct?

Reported by helder.pereira.correia on 2014-05-08 21:05:47

[Daniel Serodio]

As the original bug reported, let me chime in. My impression is that the digits grouping
is happening right-to-left in the decimal part, while it should be right-to-left in
the integer part and left-to-right in the decimal part.

In the examples in comment #13 I can see that it works for period (12 345.123 45) but
not for comma (12 345,12 345).

Reported by dserodio on 2014-05-08 21:18:21

[Former user]

The problem is that you inverted the expected behavior with the experienced behavior
in your bug report :) But as you did use "should" in the experienced behavior description,
it was clear to me which description belong to which behavior.

It's a valid bug report then, but misleading. Commit 689e787 should fixed it.

Reported by [e-mail] on 2014-05-08 21:28:59

[Former user]

Thanks for chiming in. I now understand what happened. You misplaced experienced vs.
expected (although identifying the correct behavior at the same time):

"
Expected behavior: The output is 0,33 333 333 333 333 333 333
Experienced behavior: The output should be 0,333 333 333 333 333 333 33
"

Good that everything's clear now and we have a patch that works.

Reported by helder.pereira.correia on 2014-05-08 21:30:00

[Daniel Serodio]

Oh, you're absolutely right. Sorry for the confusion :)

Reported by dserodio on 2014-05-08 21:31:21

[Former user]

Fixed in revision f335049.

Reported by helder.pereira.correia on 2014-05-09 20:50:46 - Status changed: Fixed

[Former user]

Issue 526 has been merged into this issue.

Reported by helder.pereira.correia on 2014-09-26 20:02:19

[Helder Correia]

Issue #603 was marked as a duplicate of this issue.