D28765: KSettings::Dialog: add support for KPluginInfos without a KService

dfaure added a comment. https://invent.kde.org/frameworks/kcmutils/-/merge_requests/2 REPOSITORY R295 KCMUtils REVISION DETAIL https://phabricator.kde.org/D28765 To: dfaure, pino, broulik, mart, davidedmundson, ngraham, svuorela Cc: ahmadsamir, rikmills, wbauer, kossebau, svuorela,

D28765: KSettings::Dialog: add support for KPluginInfos without a KService

ahmadsamir added a comment. I guess a proper fix will need to revert part of this to have the KModuleInfo ctor that took a KService not use KPluginInfo internally; better still, of course, is having the KPluginInfo ctor work for the case of X-KDE-ServiceTypes=SystemSettingsExternalApp, (I

D28765: KSettings::Dialog: add support for KPluginInfos without a KService

ahmadsamir added a comment. systemsettings uses KCModuleInfo::service() to check whether the moduleinfo is valid[1], the problem is creating a KPluginInfo from the KService based on a .desktop file with X-KDE-ServiceTypes=SystemSettingsExternalApp fails because it doesn't seem to have valid

D28765: KSettings::Dialog: add support for KPluginInfos without a KService

rikmills added a comment. In D28765#673191 , @wbauer wrote: > This caused a crash in openSUSE: > https://bugs.kde.org/show_bug.cgi?id=421566 The same crash can been seen in Kubuntu when trying to launch the external

D28765: KSettings::Dialog: add support for KPluginInfos without a KService

wbauer added a comment. This caused a crash in openSUSE: https://bugs.kde.org/show_bug.cgi?id=421566 REPOSITORY R295 KCMUtils REVISION DETAIL https://phabricator.kde.org/D28765 To: dfaure, pino, broulik, mart, davidedmundson, ngraham, svuorela Cc: wbauer, kossebau, svuorela, cblack,

D28765: KSettings::Dialog: add support for KPluginInfos without a KService

kossebau added inline comments. INLINE COMMENTS > dfaure wrote in kcmoduleinfo.h:131 > It's complicated. > > 1. If you use the QString constructor, you know service() is usable. That's > the case for all users of KCModuleInfo except KCModuleLoader. [Not that there > are many] > > 2. Even

D28765: KSettings::Dialog: add support for KPluginInfos without a KService

dfaure closed this revision. REPOSITORY R295 KCMUtils REVISION DETAIL https://phabricator.kde.org/D28765 To: dfaure, pino, broulik, mart, davidedmundson, ngraham, svuorela Cc: svuorela, cblack, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D28765: KSettings::Dialog: add support for KPluginInfos without a KService

dfaure added a comment. Thanks for the review! I was getting desperate to get one ;-) INLINE COMMENTS > svuorela wrote in kcmoduleinfo.cpp:73 > At a later point, I*m not sure what the purpose is for these members are - > but that's probably for another changeset. Right, I was wondering the

D28765: KSettings::Dialog: add support for KPluginInfos without a KService

svuorela accepted this revision. svuorela added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > kcmoduleinfo.cpp:73 > +lib = pluginInfo.libraryPath(); > +keywords = > pluginInfo.property(QStringLiteral("Keywords")).toStringList(); > } At a later

D28765: KSettings::Dialog: add support for KPluginInfos without a KService

dfaure added a comment. Any idea who could review this? REPOSITORY R295 KCMUtils REVISION DETAIL https://phabricator.kde.org/D28765 To: dfaure, pino, broulik, mart, davidedmundson, ngraham Cc: cblack, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D28765: KSettings::Dialog: add support for KPluginInfos without a KService

dfaure added a reviewer: ngraham. REPOSITORY R295 KCMUtils REVISION DETAIL https://phabricator.kde.org/D28765 To: dfaure, pino, broulik, mart, davidedmundson, ngraham Cc: cblack, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D28765: KSettings::Dialog: add support for KPluginInfos without a KService

dfaure added a dependent revision: D28848: Port kontact plugin loading to KPluginLoader. REPOSITORY R295 KCMUtils REVISION DETAIL https://phabricator.kde.org/D28765 To: dfaure, pino, broulik, mart, davidedmundson Cc: cblack, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D28765: KSettings::Dialog: add support for KPluginInfos without a KService

dfaure added a comment. For KF6 we could also just replace KCModuleInfo with KPluginInfo. It really doesn't provide anything on top REPOSITORY R295 KCMUtils REVISION DETAIL https://phabricator.kde.org/D28765 To: dfaure, pino, broulik, mart, davidedmundson Cc: cblack,

D28765: KSettings::Dialog: add support for KPluginInfos without a KService

dfaure updated this revision to Diff 79906. dfaure added a comment. Use @warning. Thanks for the tip! REPOSITORY R295 KCMUtils CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28765?vs=79893=79906 BRANCH master REVISION DETAIL https://phabricator.kde.org/D28765 AFFECTED

D28765: KSettings::Dialog: add support for KPluginInfos without a KService

cblack added inline comments. INLINE COMMENTS > kcmoduleinfo.h:130 > * @return a QExplicitlySharedDataPointer to KService created from the > modules .desktop file > + * WARNING: this will be null if this KCModuleInfo was created from a > KPluginInfo coming from KPluginMetaData > +

D28765: KSettings::Dialog: add support for KPluginInfos without a KService

dfaure created this revision. dfaure added reviewers: pino, broulik, mart, davidedmundson. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. dfaure requested review of this revision. REVISION SUMMARY KPluginInfo evolved into an abstraction over old-style