Fixes group separated stored constants that would show as nan
AcceptedPublic

Authored by Joefish on Jul 4 2019, 9:47 PM.

Details

Summary
The integer_regex in KNumber(const QString &s) does not recognize
numbers with group separators. Therefore, they are removed from the
string representation of the number before being passed.
Also, sanitizing the decimal separator is not needed as it is
already done in the KNumber constructor.
Test Plan
(Copied from @Dan that reported the bug)
1. In Settings select Science Mode
2. In Settings turn on Constants Buttons
3. With a number greater than one thousand in the display field
   (with comma separators) select the Shift button.
   The C1-C6 buttons will all change to Store
4. Select one of the Store buttons
5. Clear the display with AC button
6. Hover over the button the number was stored in (C1-C6).
   The stored number should be displayed as a tool tip
7. Select the button the number was stored in

BUG: 408752

Diff Detail

Repository
R353 KCalc
Branch
bug_408752 (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 13650
Build 13668: arc lint + arc unit
Joefish created this revision.Jul 4 2019, 9:47 PM
Restricted Application added a subscriber: kde-utils-devel. · View Herald TranscriptJul 4 2019, 9:47 PM
Joefish requested review of this revision.Jul 4 2019, 9:47 PM
Joefish updated this revision to Diff 61185.Jul 4 2019, 10:54 PM

Checks if group separators are enabled

Another possibility would be to add the removal of the group separators to the KNumber constructor as decimal point conversion is already done there.
I wrote it like this because it is the most non-invasive way and stored constants seem to be the only part affected by it.

cfeck added a subscriber: cfeck.Jul 5 2019, 2:00 AM

If the constants are stored in localized format, does changing between locales that swap the decimal separators cause issues?

If the constants are stored in localized format, does changing between locales that swap the decimal separators cause issues?

Yes, it shows as 'nan'. Replacing the decimal point with "." as a unified way to store the value as it was done before
doesn't work with the group separator removal if they are equivalent.
I guess the comments I removed were referring to that problem. I assumed KNumber could only take sanitized inputs to C locale
so I just removed them..

Joefish updated this revision to Diff 61215.EditedJul 5 2019, 12:15 PM

Constants are stored in a C locale way and converted to the current locale
settings before being passed to the KNumber constructor.
Unfortunately, the tooltip doesn't show the stored constant in the active
locale as no locale information is stored with the number so that stored constants
between KCalc instances with different locales could be converted on startup.

Joefish updated this revision to Diff 61229.Jul 5 2019, 4:21 PM

Added const qualifier to decimalPoint, groupSeparator and groupSeparatorEnabled

This revision is now accepted and ready to land.Dec 29 2019, 12:00 AM