D29201: KCMUtils: Add option to append service file to list of arguments

2020-05-19 Thread Alexander Lohnau
alex abandoned this revision. alex added a comment. Abandoning this in favor of https://invent.kde.org/frameworks/kconfigwidgets/-/merge_requests/1 and a follow up PR. REPOSITORY R295 KCMUtils REVISION DETAIL https://phabricator.kde.org/D29201 To: alex, #plasma, ngraham, meven,

D29201: KCMUtils: Add option to append service file to list of arguments

2020-05-19 Thread Alexander Lohnau
alex added inline comments. INLINE COMMENTS > meven wrote in kpluginselector.cpp:855 > Seems like a property would make sense, after all it is about it, or a ref to > the KCModuleInfo I have created a PR on Gitlab for this REPOSITORY R295 KCMUtils REVISION DETAIL

D29201: KCMUtils: Add option to append service file to list of arguments

2020-05-19 Thread Méven Car
meven added inline comments. INLINE COMMENTS > alex wrote in kpluginselector.cpp:855 > > moduleInfo is part of the ctor here, so the fileName is already available > > indirectly. > > Yes, but the KCModuleProxy is just a wrapper class for the KCModule. > The actual KCModule gets created in

D29201: KCMUtils: Add option to append service file to list of arguments

2020-05-16 Thread Alexander Lohnau
alex added inline comments. INLINE COMMENTS > meven wrote in kpluginselector.cpp:855 > moduleInfo is part of the ctor here, so the fileName is already available > indirectly. > A good thing would be to avoid having an option for such a niche usage > although legitimate. > moduleInfo is part

D29201: KCMUtils: Add option to append service file to list of arguments

2020-05-16 Thread Méven Car
meven added inline comments. INLINE COMMENTS > alex wrote in kpluginselector.cpp:855 > You could add a field to the KCModule class (thats where you ultimately want > to access the fileName). > But then the downside is that you don't have them as a constructor argument. moduleInfo is part of

D29201: KCMUtils: Add option to append service file to list of arguments

2020-05-13 Thread Alexander Lohnau
alex added inline comments. INLINE COMMENTS > meven wrote in kpluginselector.cpp:855 > Adding a fileName field to KCModuleProxy would make more sense to me, and do > it by default. > Plus KCModuleProxy has already access to the fileName since it receives > moduleInfo. > But I am not a

D29201: KCMUtils: Add option to append service file to list of arguments

2020-05-13 Thread Méven Car
meven added inline comments. INLINE COMMENTS > kpluginselector.cpp:855 > +} > +KCModuleProxy *currentModuleProxy = new > KCModuleProxy(moduleInfo, moduleProxyParentWidget, arguments); > if (currentModuleProxy->realModule()) { Adding a fileName field to

D29201: KCMUtils: Add option to append service file to list of arguments

2020-05-12 Thread Alexander Lohnau
alex added a comment. Friendly Ping  REPOSITORY R295 KCMUtils REVISION DETAIL https://phabricator.kde.org/D29201 To: alex, #plasma, ngraham, meven, broulik, mart Cc: mart, apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29201: KCMUtils: Add option to append service file to list of arguments

2020-05-04 Thread Alexander Lohnau
alex updated this revision to Diff 81873. alex added a comment. Increase @since to 5.71 REPOSITORY R295 KCMUtils CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29201?vs=81856=81873 BRANCH service_path_append (branched from master) REVISION DETAIL

D29201: KCMUtils: Add option to append service file to list of arguments

2020-05-04 Thread Alexander Lohnau
alex added a reviewer: mart. REPOSITORY R295 KCMUtils REVISION DETAIL https://phabricator.kde.org/D29201 To: alex, #plasma, ngraham, meven, broulik, mart Cc: mart, apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29201: KCMUtils: Add option to append service file to list of arguments

2020-05-04 Thread Alexander Lohnau
alex updated this revision to Diff 81856. alex added a comment. Linebreak REPOSITORY R295 KCMUtils CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29201?vs=81855=81856 BRANCH service_path_append (branched from master) REVISION DETAIL https://phabricator.kde.org/D29201

D29201: KCMUtils: Add option to append service file to list of arguments

2020-05-04 Thread Alexander Lohnau
alex updated this revision to Diff 81855. alex added a comment. Improve documentation Is that what you had in mind :-) ? And should I maybe add a comment to make this the default in KF6? REPOSITORY R295 KCMUtils CHANGES SINCE LAST UPDATE

D29201: KCMUtils: Add option to append service file to list of arguments

2020-05-04 Thread Marco Martin
mart added a comment. indeed, a bit more documentation then go for it REPOSITORY R295 KCMUtils REVISION DETAIL https://phabricator.kde.org/D29201 To: alex, #plasma, ngraham, meven, broulik Cc: mart, apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29201: KCMUtils: Add option to append service file to list of arguments

2020-05-03 Thread Alexander Lohnau
alex added a comment. > Would it make sense to always pass it? Would it regress in any way? It would make sense, but it would change the existing behavior. If you use the method setConfigurationArguments then you expect your argument list to be the list of arguments you specified + an

D29201: KCMUtils: Add option to append service file to list of arguments

2020-05-03 Thread Aleix Pol Gonzalez
apol added a comment. I have the feeling there's a missing piece here. Would it make sense to always pass it? Would it regress in any way? At the very least, on the API documentation, explain why one would want that. REPOSITORY R295 KCMUtils REVISION DETAIL

D29201: KCMUtils: Add option to append service file to list of arguments

2020-05-03 Thread Alexander Lohnau
alex added a dependent revision: D29384: KCM Runners: Use setAppendServiceFile method for plugin selector. REPOSITORY R295 KCMUtils REVISION DETAIL https://phabricator.kde.org/D29201 To: alex, #plasma, ngraham, meven, broulik Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham,

D29201: KCMUtils: Add option to append service file to list of arguments

2020-05-03 Thread Alexander Lohnau
alex updated this revision to Diff 81799. alex added a comment. Little typo :-) REPOSITORY R295 KCMUtils CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29201?vs=81798=81799 BRANCH service_path_append (branched from master) REVISION DETAIL https://phabricator.kde.org/D29201

D29201: KCMUtils: Add option to append service file to list of arguments

2020-05-03 Thread Alexander Lohnau
alex updated this revision to Diff 81798. alex added a comment. Rename method REPOSITORY R295 KCMUtils CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D29201?vs=81239=81798 BRANCH service_path_append (branched from master) REVISION DETAIL https://phabricator.kde.org/D29201

D29201: KCMUtils: Add option to append service file to list of arguments

2020-05-03 Thread Alexander Lohnau
alex edited the summary of this revision. alex added reviewers: meven, broulik. REPOSITORY R295 KCMUtils REVISION DETAIL https://phabricator.kde.org/D29201 To: alex, #plasma, ngraham, meven, broulik Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29201: KCMUtils: Add option to append service file to list of arguments

2020-04-26 Thread Alexander Lohnau
alex created this revision. alex added reviewers: Plasma, ngraham. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. alex requested review of this revision. REVISION SUMMARY By setting the new variable to true the service path gets appended to the list of