Bug 433730

Summary: krunner calculator lacks floating point number precision when compiled without qalculate support
Product: [Plasma] krunner Reporter: Zoltan Puskas <zoltan>
Component: calculatorAssignee: Alexander Lohnau <alexander.lohnau>
Status: RESOLVED FIXED    
Severity: normal CC: asturm, nate, plasma-bugs
Priority: NOR    
Version: 5.21.0   
Target Milestone: ---   
Platform: Gentoo Packages   
OS: Linux   
Latest Commit: Version Fixed In: 5.24
Sentry Crash Report:

Description Zoltan Puskas 2021-02-28 06:06:03 UTC
SUMMARY


STEPS TO REPRODUCE
1. Alt-F2 to call krunner
2. 6729.69-6727.6


OBSERVED RESULT
2.0899999999992

EXPECTED RESULT
2.09


SOFTWARE/OS VERSIONS
Linux/KDE Plasma: Gentoo Linux, kernel-5.11.0, updated to 2021-02-22
(available in About System)
KDE Plasma Version: 5.21.0
KDE Frameworks Version: 5.79.0
Qt Version: 5.15.2

ADDITIONAL INFORMATION

I've noticed this sometime late 2014, so this has been around for many versions.  If I use the KCalc application, I get correct result. I'd expect the calculator plugin for krunner to produce the same results.
Comment 1 Alexander Lohnau 2021-02-28 08:15:07 UTC
It seems like you don't have qalculate installed. With that you get the expected result. Otherwise QJSEngine is used to evaluate the result.
Comment 2 Nate Graham 2021-03-02 18:07:25 UTC
Hmm, Should we mark this as INTENTIONAL an require qalculate as a hard dependency, or fix the built-in calculation? My preference would be for the former.
Comment 3 Andreas Sturmlechner 2021-03-02 20:08:31 UTC
The downstream plasma-workspace package correctly lists sci-libs/libqalculate as an optional dependency. We can certainly enable that by default, and improve the description in case user is disabling the option.
Comment 4 Alexander Lohnau 2021-03-02 20:20:25 UTC
When you run `node` and type in the expression you mentioned it you get a similar result. Messing with the JS engine would seem like a really bad idea.

I personally have been thinking that it could make sense to not build this runner at all when qaclulate is not available, it is kindof confusing if you still have the runner, but it does not work as you would expect.
Comment 5 Zoltan Puskas 2021-04-17 03:18:04 UTC
Indeed, enabling the +qalculate USE flag for kde-plasma/plasma-workspace does resolve the issue, not sure why I've not noticed this before. As far as I can tell this flag has been around for some years now.

I will work with Andreas to have this enabled by default in Gentoo.
Comment 6 Nate Graham 2021-04-17 13:58:02 UTC
Should we maybe make qalculate mandatory? If not using it causes this problem, that doesn't seem ideal.
Comment 7 Andreas Sturmlechner 2021-04-18 17:10:03 UTC
(In reply to Alexander Lohnau from comment #4)
> I personally have been thinking that it could make sense to not build this
> runner at all when qaclulate is not available, it is kindof confusing if you
> still have the runner, but it does not work as you would expect.
^ second that. I'd say build the runner conditionally on qalculate.
Comment 8 Bug Janitor Service 2021-05-10 01:29:16 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/856
Comment 9 Nate Graham 2022-01-01 01:00:46 UTC
Git commit 4c5e454024df728885f42e9757e35d993afa948a by Nate Graham, on behalf of Zoltan Puskas.
Committed on 01/01/2022 at 00:52.
Pushed by ngraham into branch 'master'.

calculatorrunner: Enforce using Qalculate

If libqalculate is not present on the system, calculator runner falls
back to using QJSEngine, which results in significantly reduced
precision. Make calculator runner unconditionally dependent on
Qalculate and remove conditional fallback code to QJSEngine.

M  +5    -0    CMakeLists.txt
M  +18   -36   runners/calculator/CMakeLists.txt
M  +0    -5    runners/calculator/autotests/calculatorrunnertest.cpp
M  +0    -179  runners/calculator/calculatorrunner.cpp
M  +0    -8    runners/calculator/calculatorrunner.h

https://invent.kde.org/plasma/plasma-workspace/commit/4c5e454024df728885f42e9757e35d993afa948a