Allow shortcut keys for both decimal separators point and comma
ClosedPublic

Authored by apichlkostner on Sep 21 2018, 2:01 PM.

Details

Summary

Re-enable commented out code to handle both '.' and ',' shortcut keys for the decimal separator. Also port it from KLocale to QLocale.

BUG: 357824

Test Plan

Running kcalc with both locale en_US.UTF-8 and de_DE.UTF-8.
In both cases the keys '.' and ',' work.

Diff Detail

Repository
R353 KCalc
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
apichlkostner created this revision.Sep 21 2018, 2:01 PM
Restricted Application added a subscriber: kde-utils-devel. · View Herald TranscriptSep 21 2018, 2:01 PM
apichlkostner requested review of this revision.Sep 21 2018, 2:01 PM
cfeck requested changes to this revision.Sep 21 2018, 6:35 PM
cfeck added inline comments.
kcalc.cpp
374

QLocale::decimalPoint() returns a QChar, not a QString.

This revision now requires changes to proceed.Sep 21 2018, 6:35 PM
cfeck added a comment.Sep 21 2018, 6:45 PM

What happens if the locale uses . as a decimal point, and , as a thousands separator (or vice versa)?

Conversion of '.' and ',' to QLatin1Char instead of QLatin1String.

apichlkostner added a comment.EditedSep 21 2018, 9:53 PM

What happens if the locale uses . as a decimal point, and , as a thousands separator (or vice versa)?

I saw the other patch for converting strings which are pasted to kcalc where it may cause problems since the meaning of separators in the string is not clear.
But this patch only adds another shortcut key. The digit group separator should normally not be entered manually but only shown automatically to help the user.

Other calculators also allow to use both "." and "," as decimal separator (e.g. galculator and calc from Windows), so this behavior should be fine.
For me the main problem with the current behavior is that in all programming languages the dot is used and I automatically use the dot also in the calculator. Maybe accepting both separators is also useful in other situations.

apichlkostner marked an inline comment as done.Sep 21 2018, 9:59 PM
cfeck accepted this revision.Sep 26 2018, 8:33 PM
This revision is now accepted and ready to land.Sep 26 2018, 8:33 PM
cfeck edited the summary of this revision. (Show Details)Oct 2 2018, 2:54 AM
cfeck added a comment.Oct 2 2018, 2:58 AM

Arthur, I didn't find any previous KDE patches from you. To commit the change, I would need your mail address for proper attribution.

Arthur, I didn't find any previous KDE patches from you. To commit the change, I would need your mail address for proper attribution.

Yes, that's my first KDE patch. My email address is apichlkostner@gmx.de

This revision was automatically updated to reflect the committed changes.