Differential D6899

support X-KDE-OnlyShowOnQtPlatforms
ClosedPublic
Actions

Authored by mart on Jul 25 2017, 2:10 PM.

Details

Reviewers
davidedmundson
Group Reviewers
Frameworks
Plasma
Commits
R297:937ade5d7357: support X-KDE-OnlyShowOnQtPlatforms
Summary

some kded modules can only run in one platform,
especially only xcb or only wayland. this
makes kded support the X-KDE-OnlyShowOnQtPlatforms
entry in the desktop file

Test Plan

touchpad and keyboard kdeds not loaded in wayland

Diff Detail

Repository
R297 KDED
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mart created this revision.Jul 25 2017, 2:10 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptJul 25 2017, 2:10 PM
Restricted Application added a subscriber: Frameworks. · View Herald Transcript
broulik added a subscriber: broulik.Jul 25 2017, 2:19 PM
broulik added inline comments.
src/kded.cpp
328
KPluginMetaData::readStringList(module.rawData(), QStringLiteral("X-KDE-OnlyShowOnQtPlatforms"));
330

isEmpty()

src/kded.h
97

static?

src/kdedmodule.desktop
111 ↗(On Diff #17175)

QStringList

broulik added a comment.Jul 25 2017, 2:20 PM

Also, your property is X-KDE-PluginInfo-OnlySupportedPlatform but you check for X-KDE-OnlyShowOnQtPlatforms

mart added a comment.Jul 25 2017, 2:22 PM

ah, yeah, that's a leftover, should go

mart updated this revision to Diff 17176.Jul 25 2017, 2:23 PM

readstringlist

Restricted Application added a project: Plasma. · View Herald TranscriptJul 25 2017, 2:23 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
mart updated this revision to Diff 17177.Jul 25 2017, 2:24 PM

less blanks

mart marked 2 inline comments as done.Jul 25 2017, 2:24 PM
dfaure added a subscriber: dfaure.Aug 8 2017, 7:22 AM

"Show" is a bit of a historical name here, this isn't about showing apps in a menu...

X-KDE-OnlyLoadOnQtPlatforms would be closer to the truth, no?

Looks good otherwise.

mart added a comment.Aug 8 2017, 9:24 AM
In D6899#133625, @dfaure wrote:

"Show" is a bit of a historical name here, this isn't about showing apps in a menu...

X-KDE-OnlyLoadOnQtPlatforms would be closer to the truth, no?

Looks good otherwise.

that would be a new key, would you prefer that version?

dfaure added a comment.Aug 8 2017, 11:28 AM

Ah, this key is already used, for applications? I wasn't aware, if that's the case.

Hmm, then maybe better keep the same key indeed.

davidedmundson accepted this revision.Aug 14 2017, 9:35 AM
This revision is now accepted and ready to land.Aug 14 2017, 9:35 AM
Closed by commit R297:937ade5d7357: support X-KDE-OnlyShowOnQtPlatforms (authored by mart). · Explain WhyAug 14 2017, 10:25 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Msrc/kded.h (7 lines)
Msrc/kded.cpp (10 lines)

Diff 18132

View Options

src/kded.h

Loading...
View Options

src/kded.cpp

Loading...