D10078: Add separate lib KF5::DBusRunner

[Friedrich W. H. Kossebau]

kossebau added inline comments.

INLINE COMMENTS

> broulik wrote in dbusadaptor_p.h:1
> Isn't this file compile-time generated from xml?

It can be, yes. I had switched for reason* to  manually written one, but with the public class API no longer affected by reason* I could/should perhaps switch back.
Nothing really won or lost though by having a non-generated class.

*qdbusxml2cpp picks up the casing of the method names in the D-Bus interface and expects them also to be used in the wrapped class. D-Bus method naming standard seems to be "UpperCase", which also used in the krunner1 interface ("Actions", "Run", "Query").
While in our Qt-style code we prefer method names to be with lowerCaseStart.
And given the purpose of the lib is "being nice&easy to use", I switched to a manually adapted version of the adaptor code. Compare e.g. changed to API of TestRemoteRunner.
Later though, when I went to use QDBusContext for enabling the async match collection, I avoided leaking that detail in the public API and moved all D-Bus stuff to the pimpl object. Where having "UpperCaseStart" methods is not harming public consumers.
Whatever people prefer, no own preference.

> broulik wrote in querymatch.h:35
> Should be pimpl this? once we commit to this we can never add new fields but then the DBus signature is also now or never.. I think we should add subtext and url to the main struct since I always use the two. Originally David and I thought we could put them in properties as they're seldom used but turns out they're not.

Indeed, DBus signature being fixed is also why I wrote the simple struct for now instead of setter/getter pimpl API.
Having learned how to use c++11 initlalizer lists recently I also now fancy their use where possible ;) and in the little sample cpde I played for now using initializer lists worked nicely, no need for explicit setters. No getters used even. After all the runners are just producing that content usually and do no own further processing, but just deliver it.

Putting subtext and url to the main struct if you like. No idea yet how to adapt the marshalling, given those two need to be in the a{sv}, but possibly can be done with some temporary variantmap. No big runtime costs, and will be nicer API.

No strong opinion here as well. You decide and I will write :)

> broulik wrote in querymatch.h:53
> Can we static assert this in the dbus runner somehow? But I guess since it's a separate lib you cannot

No idea yet how to automatically ensure this :/ Why are there  no guides with tips & tricks for D-Bus interface types?!

REPOSITORY
  R308 KRunner

REVISION DETAIL
  https://phabricator.kde.org/D10078

To: kossebau, davidedmundson, broulik
Cc: ngraham, #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180124/9140f2bc/attachment.html>