[Baloo KCM] Add the ability to suspend, resume, and monitor indexing
Needs ReviewPublic

Authored by ngraham on Nov 21 2019, 5:59 PM.

Details

Reviewers
bruns
davidedmundson
Group Reviewers
VDG
Plasma
Baloo
Summary

Most of this code is lifted straight from the file indexing monitor KInfoCenter KCM.
If and when this lands, that becomes largely redundant and we could consider removing it.

Depends on D25743

FEATURE: 374474
FEATURE: 405307
FIXED-IN: 5.19.0

Test Plan

Everything seems to work.

Diff Detail

Repository
R119 Plasma Desktop
Branch
suspend-and-resume-and-monitor-indexing-from-the-kcm (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 22384
Build 22402: arc lint + arc unit
ngraham created this revision.Nov 21 2019, 5:59 PM
Restricted Application added a project: Plasma. · View Herald TranscriptNov 21 2019, 5:59 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ngraham requested review of this revision.Nov 21 2019, 5:59 PM

For the concept a big +1 from me. I did not even know that we had that in the infocenter...

For the concept a big +1 from me. I did not even know that we had that in the infocenter...

I think most people don't know this, because it's an odd place to have controls to start and stop indexing. If and when this patch lands, I think we could consider removing the Baloo page entirely.

davidedmundson added inline comments.
kcms/baloo/package/contents/ui/constants.js
23 ↗(On Diff #70119)

Why doesn't this come from the plugin?

kcms/baloo/package/contents/ui/main.qml
111

Is the space after %1 deliberate?

ngraham updated this revision to Diff 70859.Dec 3 2019, 9:02 PM
ngraham marked an inline comment as done.

Correct string error

davidedmundson added inline comments.Dec 4 2019, 12:34 AM
kcms/baloo/package/contents/ui/main.qml
93

what about:

LowPowerIdle?

ngraham updated this revision to Diff 70904.Dec 4 2019, 2:57 PM
ngraham marked an inline comment as done.

ALso check for LowPowerIdle state

ngraham added inline comments.Dec 4 2019, 3:11 PM
kcms/baloo/package/contents/ui/constants.js
23 ↗(On Diff #70119)

I'm not sure exactly what that means or how to do it; can you help me with it?

davidedmundson requested changes to this revision.Dec 4 2019, 3:21 PM
davidedmundson added inline comments.
kcms/baloo/package/contents/ui/constants.js
23 ↗(On Diff #70119)

See D25743

Code here would become Baloo.Idle

This revision now requires changes to proceed.Dec 4 2019, 3:21 PM
ngraham edited the summary of this revision. (Show Details)Dec 4 2019, 3:28 PM
ngraham updated this revision to Diff 70990.Dec 5 2019, 9:45 PM

Update to work with D25743

davidedmundson added inline comments.Dec 5 2019, 9:54 PM
kcms/baloo/package/contents/ui/main.qml
82

This is the wrong way round

davidedmundson added inline comments.Dec 6 2019, 1:03 AM
kcms/baloo/package/contents/ui/main.qml
82

Edit, no it wasn't wrong

But I have found what is.

I export this as the class Baloo, but the import is also under Baloo

So for this to work against my patch it would be Baloo.Baloo.Suspended

ngraham updated this revision to Diff 70998.Dec 6 2019, 2:53 AM
ngraham marked 2 inline comments as done.

Adjust to D25743 some more

ngraham edited the test plan for this revision. (Show Details)Dec 6 2019, 2:54 AM

Thanks @davidedmundson. There's one more FIXME in the code that I could use your help with, if you don't mind.

davidedmundson added inline comments.Dec 6 2019, 6:34 AM
kcms/baloo/package/contents/ui/main.qml
94

Monitor's property is called state. Lowercase S.

A switch statement here might make this a bit easier to read.

ngraham updated this revision to Diff 71013.Dec 6 2019, 1:44 PM

Address review comments

ngraham added inline comments.Dec 6 2019, 1:45 PM
kcms/baloo/package/contents/ui/main.qml
94

Done, but now opening the KCM prints a ton of these errors:

QMetaProperty::read: Unable to handle unregistered datatype 'Baloo::IndexerState' for property 'Baloo::Monitor::state'
ngraham marked an inline comment as done.Dec 6 2019, 1:45 PM

Done, but now opening the KCM prints a ton of these errors:

Is this still an issue?

ngraham updated this revision to Diff 73771.Jan 17 2020, 2:51 PM

Adjust to changes in D25743

ngraham edited the summary of this revision. (Show Details)Jan 17 2020, 2:51 PM
davidedmundson added inline comments.Jan 17 2020, 2:53 PM
kcms/baloo/package/contents/ui/main.qml
82

So for this to work against my patch it would be Baloo.Baloo.Suspended

This isn't in this diff

ngraham marked an inline comment as done.Jan 17 2020, 3:15 PM
ngraham updated this revision to Diff 73775.Jan 17 2020, 3:24 PM

Re-add namespace

ngraham marked an inline comment as done.Jan 17 2020, 3:24 PM

Re-added the namespace but the state checking still doesn't seem to work properly.

That turned out to be an unrelated Baloo bug that was recently fixed by D27326.

This is ready for re-review.

Wouldn't it look better to use the new header/title component for the list view ?

Hmm, in this case it doesn't seem appropriate to me.