Bug 406388 - Calculator doesn't work well on decimal numbers with es_PE locale.
Summary: Calculator doesn't work well on decimal numbers with es_PE locale.
Status: RESOLVED FIXED
Alias: None
Product: krunner
Classification: Plasma
Component: calculator (show other bugs)
Version: 5.15.4
Platform: Neon Linux
: NOR normal
Target Milestone: ---
Assignee: Alexander Lohnau
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-04-10 05:36 UTC by Cesar Garcia
Modified: 2022-08-13 14:19 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.20


Attachments
systemsettings capture that shows dot separator for es_PE locale. (55.57 KB, image/png)
2020-07-29 19:20 UTC, Cesar Garcia
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Cesar Garcia 2019-04-10 05:36:19 UTC
SUMMARY

This bug is very similar to #304442 but with inverse locale rules. My locale uses point to separate decimals, for example 123.45 but krunner calculator plugin gives the results on the wrong locale (using comma, like 123,45), thus making the result unusable to continue doing calculations.

STEPS TO REPRODUCE
1. In krunner enter =12.50+25.06
2. Press <enter>
3. 

OBSERVED RESULT
The calculator result shows 37,56 and upon pressing enter the result shows [37;56] (the edit area changes to 37,56).

EXPECTED RESULT
The calculator result should show 37.56 and upon pressing enter the result dissappears (the edit area should change to 37.56).

SOFTWARE/OS VERSIONS
KDE Neon: 5.15
KDE Plasma Version: 5.15.4
KDE Frameworks Version: 5.56.0
Qt Version: 5.12.0

ADDITIONAL INFORMATION

My locale is es_PE.UTF-8, but any locale that uses dot for decimal separator should work.
Comment 1 2wxsy58236r3 2020-07-08 00:47:13 UTC
I cannot reproduce the issue with ja_JP.UTF-8 (which uses dot for decimal separator), Plasma 5.19.2, libqalculate 3.11.0.

Bug reporter, can you please try again after updating the packages?
Comment 2 Cesar Garcia 2020-07-08 01:36:09 UTC
Tried it again but the bug is still present.

KDE Neon: 5.19
KDE Plasma Version: 5.19.3
KDE Frameworks Version: 5.71.0
Qt Version: 5.14.2

The only difference is that the libqalculate version provided by the distro is older than yours.

libqalculate14: 2.2.1-0+18.04+bionic+build3

Did you compile libqalculate by yourself or are using a distro where a package for the latest version is provided? I could test again in a VM to check if the problem lies on a old libqalculate version of is still a kde related bug.
Comment 3 2wxsy58236r3 2020-07-08 07:25:24 UTC
I am using Arch Linux, which provides the latest version of libqalculate.

If you want to test in a VM, Manjaro is a good choice because it is based on Arch Linux and is easier to install.
Comment 4 Alexander Lohnau 2020-07-08 19:47:36 UTC
I can still reproduce this on KDE Neon Unstable with version: 2.2.1-0.

Could you maybe run:
>echo $LC_NUMERIC 
This ENV variable is used to determine the decimal separator.

Commas can also be used in the context of vectors, for example:
>[1,2]+[3,4]
So we could just check if we explicitly use vectors, and if not do pretty much the same thing as in https://invent.kde.org/plasma/plasma-workspace/-/blob/master/runners/calculator/calculatorrunner.cpp#L186 but just the other way around.
Comment 5 2wxsy58236r3 2020-07-09 00:46:10 UTC
(In reply to Alexander Lohnau from comment #4)
> I can still reproduce this on KDE Neon Unstable with version: 2.2.1-0.

I believe you need a more recent version (preferably the latest version) of libqalculate to solve the issue.
(See also: https://github.com/Qalculate/libqalculate/commit/697b99922b2457a7d18375f6c7c859592eaf38e4)

> Could you maybe run:
> >echo $LC_NUMERIC 

$ echo $LC_NUMERIC
ja_JP.UTF-8

$ locale -kc LC_NUMERIC
LC_NUMERIC
decimal_point="."
thousands_sep=","
grouping=3
numeric-decimal-point-wc=46
numeric-thousands-sep-wc=44
numeric-codeset="UTF-8"

> Commas can also be used in the context of vectors, for example:
> >[1,2]+[3,4]

For me, krunner returns [4, 6].
Comment 6 Cesar Garcia 2020-07-17 03:47:28 UTC
Tried with a Manjaro VM. Updated to the latest version offered:

KDE Plasma Version: 5.18.5
KDE Frameworks Version: 5.70.0
Qt Version: 5.15.0
libqalculate: 3.10.0

LC_NUMERIC information:

$ locale -kc LC_NUMERIC
LC_NUMERIC
decimal_point=","
thousands_sep="."
grouping=3;3
numeric-decimal-point-wc=44
numeric-thousands-sep-wc=46
numeric-codeset="UTF-8"

If i try =12.50+25.06 and press enter i get 37,56 on the edit area (and [37;56] on the resulting preview).

If i try =12,50+25,06 and press enter i get [12; 75; 6] in both areas.

So no matter the decimal separator used i end with a result that cannot be used to continue using arithmetic operations.
Comment 7 2wxsy58236r3 2020-07-17 11:49:10 UTC
(In reply to Cesar Garcia from comment #6)
What locale are you using in the Manjaro VM? es_PE.UTF-8?

Note that from the LC_NUMERIC information you posted (decimal_point=","), the decimal separator is comma.
Comment 8 Christoph Feck 2020-07-28 12:52:17 UTC
If you can provide the information requested in comment 7, please add it.
Comment 9 Cesar Garcia 2020-07-29 19:19:40 UTC
Yes, the locale was es_PE.UTF-8, but according to the format settings in systemsettings, es_PE uses dot (.) for the decimal separator so i am not sure which one kde follows.

I have a question about the calculator part in krunner? It works like kcalc where the decimal separator is always a dot or it should switch to comma based on LC_NUMERIC?
Comment 10 Cesar Garcia 2020-07-29 19:20:28 UTC
Created attachment 130494 [details]
systemsettings capture that shows dot separator for es_PE locale.
Comment 11 Alexander Lohnau 2020-07-29 20:23:44 UTC
>I have a question about the calculator part in krunner? It works like kcalc where the decimal separator is always a dot or it should switch to comma based on LC_NUMERIC?
We replace the current separator with a dot (calculatorrunner.cpp line 190):
>cmd.replace(QLocale().decimalPoint(), QLatin1Char('.'), Qt::CaseInsensitive);
So if the decimal point from LC_NUMERIC is a comma than this will get replaced internally.

>Yes, the locale was es_PE.UTF-8, but according to the format settings in systemsettings, es_PE uses dot (.) for the decimal separator so i am not sure which one kde follows.
If you define a value in the detailed settings the env variable gets overwritten. And the
$ locale -kc LC_NUMERIC
command uses the locale settings and not env variable, that might be the issue.
Comment 12 Bug Janitor Service 2020-08-13 04:33:09 UTC
Dear Bug Submitter,

This bug has been in NEEDSINFO status with no change for at least
15 days. Please provide the requested information as soon as
possible and set the bug status as REPORTED. Due to regular bug
tracker maintenance, if the bug is still in NEEDSINFO status with
no change in 30 days the bug will be closed as RESOLVED > WORKSFORME
due to lack of needed information.

For more information about our bug triaging procedures please read the
wiki located here:
https://community.kde.org/Guidelines_and_HOWTOs/Bug_triaging

If you have already provided the requested information, please
mark the bug as REPORTED so that the KDE team knows that the bug is
ready to be confirmed.

Thank you for helping us make KDE software even better for everyone!
Comment 13 Cesar Garcia 2020-08-13 18:16:28 UTC
I have made more tests and seems that the bug is in Qt. I switched my locale to Colombia (also uses the comma as decimal separator) and Krunner works  as expected. The system locale seems OK (added en_US as control)

$ LC_ALL=en_US.UTF-8 locale -kc LC_NUMERIC 
LC_NUMERIC
decimal_point="."
thousands_sep=","
grouping=3;3
numeric-decimal-point-wc=46
numeric-thousands-sep-wc=44
numeric-codeset="UTF-8"

$ LC_ALL=es_PE.UTF-8 locale -kc LC_NUMERIC
LC_NUMERIC
decimal_point=","
thousands_sep="."
grouping=3;3
numeric-decimal-point-wc=44
numeric-thousands-sep-wc=46
numeric-codeset="UTF-8"

$ LC_ALL=es_CO.UTF-8 locale -kc LC_NUMERIC
LC_NUMERIC
decimal_point=","
thousands_sep="."
grouping=3;3
numeric-decimal-point-wc=44
numeric-thousands-sep-wc=46
numeric-codeset="UTF-8"

Then made a simple Qt app to check the number formatting of Colombia and Perú (should be the same)

#include <QCoreApplication>
#include <QDebug>
#include <QLocale>

int main(int argc, char *argv[])
{
    QCoreApplication a(argc, argv);
    double number = 2565.25;

    QLocale peLocale(QLocale::Spanish, QLocale::Peru);
    QString ss = peLocale.toString(number);
    auto point = peLocale.decimalPoint();
    qDebug() << "PE Separator:" << point;
    qDebug()<<"PE formatted number: " << ss;

    QLocale coLocale(QLocale::Spanish, QLocale::Colombia);
    ss = coLocale.toString(number);
    point = coLocale.decimalPoint();
    qDebug() << "CO Separator:" << point;
    qDebug()<<"CO formatted number: " << ss;
    return 0;
}

But the results were different:

PE Separator: '.'
PE formatted number:  "2,565.25"
CO Separator: ','
CO formatted number:  "2.565,25"

I am gonna open a bug report with Qt and use es_CO for the KDE number format settings as a workaround until the bug is resolved.
Comment 14 Cesar Garcia 2020-08-13 18:39:59 UTC
Just checked other systems (Libreoffice Calc, JS engines) and they all used dot for the separator on es_PE locale. Also asked other Peruvian people and while ISO says that the comma is used everybody uses the dot so i am not sure anymore if is a bug with Qt.
Comment 15 Alexander Lohnau 2020-08-13 19:01:08 UTC
IMO it makes sense to also allow the users to also use commas, I will write a patch!

But I can't help you to decide whether or not it is a Qt bug.
Comment 16 Cesar Garcia 2020-08-13 19:21:20 UTC
I tried to find the bottom of the rabbit hole and found that Qt (and maybe others) parses the Unicode Common Locale Data Repository (CLDR) for their locale info. In 2016 Perú issued a government resolution [1] where the comma must be used as decimal separator. I found the CLDR bug tracker and there is an issue [2] where they are aware of the change but are currently using the dot as is the common usage in the country right now.

A proper fix would be that CLDR updates their data so other projects (including Qt) will use the correct separator.

This won't happen anytime soon so a patch where the comma can also be used in the krunner calculator will be a welcome change.

[1] https://www.sbn.gob.pe/Repositorio/resoluciones_sbn/RESOLUCION_559_0309.pdf
[2] https://unicode-org.atlassian.net/projects/CLDR/issues/CLDR-11048
Comment 17 Bug Janitor Service 2020-08-14 05:40:03 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/213
Comment 18 Alexander Lohnau 2020-08-31 12:26:17 UTC
Git commit a8644836253ee00f265ae11390dfd4afcef179ce by Alexander Lohnau.
Committed on 31/08/2020 at 12:26.
Pushed by alex into branch 'master'.

Allow comma when dot is decimal separator
FIXED-IN: 5.20

This allows us to use a comma as a decimal separator, even if we have a
dot configured.
Especially when there is a difference between the official and
implemented decimal separator (see bug report) or if the user switches
between settings with a different separator.
By making sure that no [ or ] character(used for vectors) is contained
before replacing the string we make sure that we still provide all the
features Qalculate does.

M  +8    -1    runners/calculator/calculatorrunner.cpp

https://invent.kde.org/plasma/plasma-workspace/commit/a8644836253ee00f265ae11390dfd4afcef179ce
Comment 19 Mahmud Nabil 2021-05-14 23:21:06 UTC
(In reply to Alexander Lohnau from comment #18)
> By making sure that no [ or ] character(used for vectors) is contained
> before replacing the string we make sure that we still provide all the
> features Qalculate does.
Hi, Qalaulate also offers a bunch of mathematical functions that accept multiple arguments(e.g. integrate, perm, comb see https://bugs.kde.org/show_bug.cgi?id=436933 for details). While qalculate always accepts ';' as a argument separator, IMO it's more convenient for users with locale which uses dot for decimal point to use ',' for argument separator.

The API reference for Calculator::calculate http://qalculate.github.io/reference/classCalculator.html#a87b613af9c067971b3a45514677fb24f suggests that
> The expression should be unlocalized first with unlocalizeExpression().
which seems do the same thing CalculatorRunner::userFriendlySubstitutions is doing now. So I built the calculator runner with  CalculatorRunner::userFriendlySubstitutions removed(when ENABLE_QALCULATE is set) and changing the line that uses Calculator::calculate at qalculate_engine.cpp to
```
MathStructure result = CALCULATOR->calculate(CALCULATOR->unlocalizeExpression(ctext), eo);
```
this seems to produce the same result as the above patch with es_PE.UTF-8 as well as not interfering with "argument separator commas" in en_US.UTF-8.

Is there any known issues or other reason that prevents the use of CALCULATOR->unlocalizeExpression in this particular case?
Comment 20 Alexander Lohnau 2022-08-04 10:52:04 UTC
>Is there any known issues or other reason that prevents the use of CALCULATOR->unlocalizeExpression in this particular case?

Sorry, I have overlooked that comment. AFAICS not, I will look into that.
Comment 21 Alexander Lohnau 2022-08-13 11:51:32 UTC
This does not seem to work when I have an English locale set and do queries with a comma as decimal separator like "4,5+5,5"
Comment 22 Mahmud Nabil 2022-08-13 14:19:50 UTC
(In reply to Alexander Lohnau from comment #21)
> This does not seem to work when I have an English locale set and do queries
> with a comma as decimal separator like "4,5+5,5"

With locale en_DK.UTF-8 the query "4,5+5,5" results "10" and
with locale en_US.UTF-8 it results a vector [4  (5 + 5)  5].
This is the exact behavior you get with the Qalculator GUI and it seems a reasonable decision since one would generally set a locale with ',' as decimal point if they are used to use ','  as decimal point.

P.S. It seems they fixed the decimal_point issue for es_PE.UTF-8, it is "." now.