KRunner fix prepare/teardown signals
ClosedPublic

Authored by alex on Apr 21 2020, 1:41 PM.

Details

Summary

BUG: 420311.
Because the teardown was requested and checked the prepped variable was set to false and consequently
the prepare method gets called for the next character typed.

The exact two lines get called in the matchSessionComplete method.
The matchSessionComplete method gets called called in RunnerResultsModel::clear (milou repo).

Test Plan

If you connect to the prepare/teardown slot, for example:

connect(this, &MyRunner::prepare, [=]() {
        qWarning() << "prepare";
});`

You see that they get triggered for each letter typed, after applying this patch they get only triggered when the match session is started/is over.

Diff Detail

Repository
R308 KRunner
Branch
krunner_signal_bugfix (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 25601
Build 25619: arc lint + arc unit
alex created this revision.Apr 21 2020, 1:41 PM
Restricted Application added a project: Frameworks. Β· View Herald TranscriptApr 21 2020, 1:41 PM
Restricted Application added a subscriber: kde-frameworks-devel. Β· View Herald Transcript
alex requested review of this revision.Apr 21 2020, 1:41 PM
alex edited the summary of this revision. (Show Details)Apr 21 2020, 1:43 PM
alex retitled this revision from WIP KRunner: Fix Bug 420311 to WIP KRunner fix prepare/teardown signals.Apr 25 2020, 2:02 PM
meven added a comment.Apr 30 2020, 6:15 AM

Try out what was described in bug report.

Please copy it the TEST PLAN (it is easier on reviewers and better for history)

Adding a test would be the best way to demonstrate the issue and the fix.

alex edited the test plan for this revision. (Show Details)Apr 30 2020, 6:18 AM
alex added a comment.May 1 2020, 5:58 AM

Adding a test would be the best way to demonstrate the issue and the fix.

I debugged this and the debugger said that the matchSessionComplete method got called somewhere from QML, but I don't know from where.
I also know only basic QML and the way the QML frontend interacts with the runner is also really confusing to me ;-).
Thats why I originally filed a bug and not directly submitted a patch.

alex added a comment.May 5 2020, 3:29 PM

How should I proceed with this patch?

cfeck added a subscriber: cfeck.

If this is no longer Work-In-Progress, please remove "WIP" from the title.

alex retitled this revision from WIP KRunner fix prepare/teardown signals to KRunner fix prepare/teardown signals.May 20 2020, 9:44 AM
alex edited the summary of this revision. (Show Details)
meven accepted this revision.May 20 2020, 4:56 PM

It is hard to understand why it was wrongly placed here, that makes this hard to review and approve.
But you are getting familiar with KRunner internals, so since it does seem benign and you tested it, I think we can merge this.

Please wait a little, to see if others have something to say, before landing.

This revision is now accepted and ready to land.May 20 2020, 4:56 PM
alex added a comment.May 22 2020, 9:33 AM

Thanks and I get why it is hard to review.

Could @broulik please help/have a look at this.
A issue I have with understanding some of the KRunner stuff is that I don't know where exactly
the runner "backend" is used (from the QML side of things).

Thanks πŸ˜ƒ

I'm sorry, I don't really know how this teardown stuff all works :/

alex added a comment.May 22 2020, 10:54 AM

But can you please explain, where in the QML code the actual KRunner backend is used?
Maybe then I can understand it better πŸ™ƒ.

the runner "backend" is used (from the QML side of things).

Repo is called milou

alex added a comment.May 22 2020, 11:02 AM

Repo is called milou

Thanks!

alex edited the summary of this revision. (Show Details)May 22 2020, 11:08 AM
alex added a comment.May 22 2020, 11:11 AM

Now I understand where it is called and how it works exactly.
May I ship this?

alex closed this revision.May 27 2020, 7:43 AM

I now have runner queries never "finish". I type "plasmashell", get the results I expect, and the busy indicator keeps spinning forever.