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
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
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
90

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 17177

View Options

src/kded.h

Loading...
View Options

src/kded.cpp

Loading...