Dictionary doesn't have enough time to complete query before resetting by milou
Needs RevisionPublic

Authored by McPain on Jul 12 2018, 7:22 AM.

Details

Summary

CCBUG: 390776

Diff Detail

Repository
R112 Milou
Lint
Lint Skipped
Unit
Unit Tests Skipped
McPain created this revision.Jul 12 2018, 7:22 AM
Restricted Application added a project: Plasma. · View Herald TranscriptJul 12 2018, 7:22 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
McPain requested review of this revision.Jul 12 2018, 7:22 AM
McPain retitled this revision from Dictionary haven't enough time to complete query before resetting by milou to Dictionary doesn't have enough time to complete query before resetting by milou.Jul 12 2018, 11:43 AM

Thanks for the patch! It will be nice to get the Dictionary runner working again.

Please change https://bugs.kde.org/show_bug.cgi?id=390776 to CCBUG: 390776. See https://community.kde.org/Infrastructure/Phabricator#Add_special_keywords

I would rather see a thorough investigation on what this does and why it is or is not needed. There's a couple of slow runners that suffer issues because of premature resetting but I don't understand why it has this code in the first place.

McPain edited the summary of this revision. (Show Details)Jul 12 2018, 2:25 PM

I would rather see a thorough investigation on what this does and why it is or is not needed. There's a couple of slow runners that suffer issues because of premature resetting but I don't understand why it has this code in the first place.

As far as I know, krunner uses milou to launch queries:

https://cgit.kde.org/milou.git/tree/lib/sourcesmodel.cpp#n215

Any movement on this? It would be great to use the dictionary in krunner. Still broken in 5.13.4.

davidedmundson requested changes to this revision.Sun, Sep 9, 7:41 AM
davidedmundson added a subscriber: davidedmundson.

No point pinging if we haven't replied to Kai's comments/question.

This revision now requires changes to proceed.Sun, Sep 9, 7:41 AM

Urgh. Just read the code.

We don't reset when we type a new letter. To do so would have a jumpy ui. Instead we reset when we get results back.

That leaves a problem when you search for "firefo" ( with 1 result) to "firefodfhhxtffrdh" with zero results. Which is what this timer "solves"

Clearly its not a good solution to the problem. Changing it to 5 seconds isn't helping.

McPain added a comment.EditedTue, Sep 11, 12:44 PM

Urgh. Just read the code.

We don't reset when we type a new letter. To do so would have a jumpy ui. Instead we reset when we get results back.

That leaves a problem when you search for "firefo" ( with 1 result) to "firefodfhhxtffrdh" with zero results. Which is what this timer "solves"

Clearly its not a good solution to the problem. Changing it to 5 seconds isn't helping.

which solution would be good?

PS I didn't understand yet why this solution isn't good enough and what things are still broken