Bug 410744 - Converter shows duplicate results when a 2nd unit is partially written
Summary: Converter shows duplicate results when a 2nd unit is partially written
Status: RESOLVED FIXED
Alias: None
Product: krunner
Classification: Plasma
Component: converter (show other bugs)
Version: 5.16.4
Platform: Ubuntu Linux
: NOR normal
Target Milestone: ---
Assignee: Kai Uwe Broulik
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-08-08 22:05 UTC by Ismael Asensio
Modified: 2019-08-10 15:48 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.16.5


Attachments
Screenshot (160.69 KB, image/png)
2019-08-08 22:05 UTC, Ismael Asensio
Details
Possible patch (242 bytes, patch)
2019-08-08 23:04 UTC, Ismael Asensio
Details
Patch to runner/converter (763 bytes, patch)
2019-08-10 07:58 UTC, Ismael Asensio
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ismael Asensio 2019-08-08 22:05:25 UTC
Created attachment 122025 [details]
Screenshot

SUMMARY
When typing a conversion, and the second unit is partially written (i.e. "10€ to ru..." most of the results are duplicates, by 2, 3 or even 5 times.

STEPS TO REPRODUCE
1. In krunner type any conversion: "10€ to "
2. Type and additional letter with several matching units "10€ to r"
3. Continue typing letters

OBSERVED RESULT
When typing a conversion, and the second unit is partially written (i.e. "10€ to ru..." most of the results are duplicates, by 2, 3 or even 5 times. This does not happen when no 2nd unit is specified nor when the 2nd unit is complete or almost complete (see attached).

EXPECTED RESULT
Each possible matching second unit is only shown one time. 

SOFTWARE/OS VERSIONS
Operating System: Kubuntu 19.04
KDE Plasma Version: 5.16.4
KDE Frameworks Version: 5.60.0
Qt Version: 5.12.2
Kernel Version: 4.20.17-042017-generic
OS Type: 64-bit

ADDITIONAL INFORMATION
This behavior happens with different categories of units (lenght, currency, ...) and on several languages (tried on es_ES and en_US)
Comment 1 Ismael Asensio 2019-08-08 23:04:47 UTC
Created attachment 122026 [details]
Possible patch

It looks like a new result is added not for every matching unit, but once for every matching stringUnit, hence the duplicates. (converterrunner.cpp, line 216)

I know the warning, but for now I find going through a Phabricator submission a little scary, so I'm leaving this 3-liner "patch" here for reference.
Comment 2 Nate Graham 2019-08-09 12:06:36 UTC
I'd be happy to help you submit it there. Since you actually have a patch file, submitting it is simply a matter of pasting it into a web page. :) 

If you don't already have a KDE Identity username and password, create one here: https://identity.kde.org/

Then use those credentials to log into https://phabricator.kde.org and paste your diff into here: https://phabricator.kde.org/differential/diff/create/
Comment 3 Ismael Asensio 2019-08-09 15:29:57 UTC
(In reply to Nate Graham from comment #2)
> I'd be happy to help you submit it there. Since you actually have a patch
> file, submitting it is simply a matter of pasting it into a web page. :) 
> 
> If you don't already have a KDE Identity username and password, create one
> here: https://identity.kde.org/
> 
> Then use those credentials to log into https://phabricator.kde.org and paste
> your diff into here: https://phabricator.kde.org/differential/diff/create/

To be honest, the "patch" is just a diff output over a single file. Although it is a very simple change I haven't even compile and run it. 
But I feel now in the mood to clone the repository, try to build from source and do a proper one. Thanks for your support!
Comment 4 Nate Graham 2019-08-09 16:23:22 UTC
You're very welcome! Let me know if I can help in any way.
Comment 5 Ismael Asensio 2019-08-10 07:58:13 UTC
Created attachment 122044 [details]
Patch to runner/converter
Comment 6 Ismael Asensio 2019-08-10 08:28:03 UTC
After more than 10 hours trying to resolve dependencies for every single package in plasma... diff is finally posted to phabricator :D 
https://phabricator.kde.org/D23064
Comment 7 Nate Graham 2019-08-10 15:48:28 UTC
Git commit 239f574c5b2fe1367406f7ccecd31f491ce46a5e by Nate Graham, on behalf of Ismael Asensio.
Committed on 10/08/2019 at 15:48.
Pushed by ngraham into branch 'Plasma/5.16'.

Fix #410744: Duplicate results when a 2nd unit is partially written in krunner

Summary:
When typing a conversion, and the second unit is partially written (i.e. "10€ to ru..." the results are duplicated by several times. This behavior happens with different categories of units (lenght, currency, ...) and on different languages.

The problem is that a new result is being added not once for every matching unit, but once for every matching stringUnit, generating the duplicates.

Before:
{F7202568}

After:
{F7202571}

This is my first attempt to build kde from source and commit the diff to phabricator, so please excuse any possible mistakes.

Test Plan:
Open krunner and type any conversion with a letter for the second unit: "10€ to r"
Result for each matching second unit is only shown one time

Reviewers: ngraham, broulik

Reviewed By: ngraham, broulik

Subscribers: plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D23064

M  +4    -1    runners/converter/converterrunner.cpp

https://commits.kde.org/kdeplasma-addons/239f574c5b2fe1367406f7ccecd31f491ce46a5e