Improve DesktopFileParser heuristic for finding old-style service desktop entries
ClosedPublic

Authored by mpyne on Sep 27 2017, 1:59 AM.

Details

Summary

When we adopted JSON plugin metadata for services, we also included compatibility code so that we could continue to load the older Desktop Entry format for KDE services (looks to me that we convert it on the fly so that the application code can just use everything as if it were from the JSON-based service metadata).

To make sure we find any legacy .desktop files for service types, we have to know their name based on their service type, so that we can load the .desktop file and add their metadata to the options we maintain for each service type definition. IIRC we could use KSycoca in kdelibs4 to find the right service type metadata, even without knowing the source desktop file entry, but we don't have KSycoca anymore so we have to look up likely .desktop files ourselves.

The heuristic we currently use maps any / characters to a -, so that a request for something like KWin/Effect service types would map to kservicetypes5/kwin-effect.desktop when searching for the .desktop file.

This heuristic doesn't find all likely desktop entry candidates though. In particular it fails for KWin/Effect (which is really at kservicetypes5/kwineffect.desktop).

This change modifies the legacy lookup code path to search first for the existing heuristic, and if that fails to search by an additional heuristic that does the exact same thing but mapping / to nothing instead.

See also bug 384037

Test Plan

Running System Settings and playing with the KWin Desktop Effects KCMs no longer spams the console with warnings about missing service types. Nothing else in the desktop seems to have been broken either. But I haven't found a much better way to test.

Diff Detail

Repository
R244 KCoreAddons
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mpyne created this revision.Sep 27 2017, 1:59 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptSep 27 2017, 1:59 AM
mpyne added a comment.Oct 1 2017, 3:18 PM

Any objections? If not I'll try to commit in the next few days.

graesslin added inline comments.
src/lib/plugin/desktopfileparser.cpp
547

Be careful with for and qt containers. This causes a detach. Either use the old variant or wrap the list in qAsConst ( which is new in 5.7)

mpyne added inline comments.Oct 4 2017, 11:20 PM
src/lib/plugin/desktopfileparser.cpp
547

OK. I think we settled on Qt 5.6 for now at the request of packagers since it's the latest Qt LTS so I'll use the old variant or manually cast to const. Thanks for the reminder!

This revision was automatically updated to reflect the committed changes.