Port away from kdelibs4support
ClosedPublic

Authored by elvisangelaccio on Aug 13 2018, 10:21 PM.

Details

Summary

It was only used as fallback when baloo was not found, but
KFileMetaDataWidget is useless without nepomuk.

The result of this patch is that the information panel and the tooltips
won't be available from platforms without baloo (instead of being
available but broken). The baloo dependency remains optional.

Closes T8720

Test Plan

Build dolphin with cmake -DCMAKE_DISABLE_FIND_PACKAGE_KF5Baloo=ON .. and
make sure it doesn't show tooltips or the information panel.

Diff Detail

Repository
R318 Dolphin
Branch
no-kdelibs4support
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 2029
Build 2047: arc lint + arc unit
Restricted Application added a project: Dolphin. · View Herald TranscriptAug 13 2018, 10:21 PM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
elvisangelaccio requested review of this revision.Aug 13 2018, 10:21 PM
rkflx accepted this revision.Aug 19 2018, 8:55 PM
rkflx added a subscriber: rkflx.

I tried your test plan, which works as advertised. Code LGTM too. Let's get rid of kdelibs4support…

One minor detail: Would it make sense to hide the Show tooltips checkbox in Dolphin's preferences too?

This revision is now accepted and ready to land.Aug 19 2018, 8:55 PM
ngraham requested changes to this revision.Aug 20 2018, 12:57 PM

+1 conceptually. Everything looks good and works well, and all the tests pass. however I agree with @rkflx and would like to see the Show Tooltips checkbox removed when Baloo support has not been compiled in. No reason to show users features that are unavailable due to distro decisions.

This revision now requires changes to proceed.Aug 20 2018, 12:57 PM

Well spotted. Would you be ok if we just disable that checkbox? (otherwise if we want to remove it, we'd need a lot more #ifdefs, which I'd like to avoid).

Oh, and if we make it disabled, we could also add a tooltip saying why.

Hmm, every UI element referencing the information panel disappears completely with no Baloo support, so I'd prefer to be consistent if possible, even if it's messier than we might prefer. :/ Is there any refactoring we could do beforehand in another commit that might make that easier?

Hmm, every UI element referencing the information panel disappears completely with no Baloo support, so I'd prefer to be consistent if possible, even if it's messier than we might prefer. :/ Is there any refactoring we could do beforehand in another commit that might make that easier?

Not really, and I get your point. Ok, we can live with some more ifdefs then.

  • Also hide 'Show tooltips' checkbox from settings dialog
ngraham accepted this revision.Aug 20 2018, 7:02 PM

Fantastic, thanks! Works great now.

This revision is now accepted and ready to land.Aug 20 2018, 7:02 PM
This revision was automatically updated to reflect the committed changes.
rkflx added a comment.Aug 21 2018, 7:12 PM

Would you be ok if we just disable that checkbox?

Sorry for not being able to respond earlier. I think the approach that landed is fine.

Fantastic, thanks! Works great now.

No, it doesn't. Now the Miscellaneous label is missing, which should be quite obvious when actually running the patch or looking at the code. I'll send a patch…