Use KIO::PreviewJob::defaultPlugins()
ClosedPublic

Authored by ngraham on Oct 17 2017, 11:00 PM.

Details

Summary

Use KIO::PreviewJob::defaultPlugins() instead of hardcoding plugin list.

Depends on D8347

Test Plan

Compiles fine in KDE Neon.

Diff Detail

Repository
R320 KIO Extras
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ngraham created this revision.Oct 17 2017, 11:00 PM

The dependent revision is now merged. Anyone object to this?

markg accepted this revision.Oct 20 2017, 3:01 PM

Please do make sure (just as a test before you push this) that the default plugins (which now is all except for the text plugin if i recall correctly) are enabled in Dolphin' settings (General -> Previews).

This revision is now accepted and ready to land.Oct 20 2017, 3:01 PM

Yep, I have tested that in D7440.

This revision was automatically updated to reflect the committed changes.
elvisangelaccio reopened this revision.Oct 21 2017, 10:24 AM

We cannot use new API without bumping the minimum KF5 version (currently 5.3, we need 5.40...)

This revision is now accepted and ready to land.Oct 21 2017, 10:24 AM
elvisangelaccio requested changes to this revision.Oct 21 2017, 10:29 AM
This revision now requires changes to proceed.Oct 21 2017, 10:29 AM

Hmm, I hadn't thought about that. 5.40 isn't released yet, so if I change set(KF5_MIN_VERSION "5.3.0") in the CMake file, it seems that this would fail to build until 5.40 is released.

Should I back this out and re-commit after KF5.40 drops?

Or here's an idea: I could use KIO::PreviewJob::availablePlugins(), which differs from the new API I've added only in that it will also enable the text previewer plugin. Then we could switch to KIO::PreviewJob::defaultPlugins() once KF5.40 is released. Does that sound reasonable?

elvisangelaccio added a comment.EditedOct 21 2017, 2:20 PM

The alternative is to wrap the change with

#if KIO_VERSION >= QT_VERSION_CHECK(5, 40, 0)

without bumping the minimum version in cmake.

ngraham updated this revision to Diff 21059.Oct 21 2017, 2:50 PM

If we don't have KDE Frameworks 5.40 yet, emulate the behavior of the KIO::PreviewJob::defaultPlugins() API that we would otherwise want to use

elvisangelaccio accepted this revision.Oct 21 2017, 4:02 PM
This revision is now accepted and ready to land.Oct 21 2017, 4:02 PM
ngraham updated this revision to Diff 21064.Oct 21 2017, 4:24 PM

Rebase against master

This revision was automatically updated to reflect the committed changes.