kdeinit related parts can be killed, the way to invoke "system email client" needs to be kept
Description
Details
- Differential Revisions
- D25549: [kcms/keyboard] Port away from KToolInvokation
Status | Assigned | Task | ||
---|---|---|---|---|
Open | None | T12171 Meta task: KService | ||
Open | None | T12185 KService: deprecate/slim down KToolInvocation | ||
Open | None | T14328 Create MailClientLauncherJob | ||
Open | dfaure | T14329 Create KTerminalLauncherJob | ||
Open | alex | T14330 Unify logic for launching KCMs |
Fleshing this task out a bit more.
KToolInvocation::startServiceByDesktopName
becomes ApplicationLaunchJob
KToolInvocation::kdeinitExec
becomes QProcess if it's something that's conceptually the same task and doesn't need startup indicators and processing .desktop files
or ApplicationLaunchJob/CommandLaunchJob if it's starting something new
the way to invoke "system email client" needs to be kept
Wouldn't a QDesktopService::openUrl("mailto:/...") suffice? It surely isn't as pretty though.
Yeah, who needs proper convenient and type-safe C++ API when they can instead mangle arguments into a string and mess up the encoding? :-)
This change in Konsole broke all the tests
https://build.kde.org/job/Applications/job/konsole/job/kf5-qt5%20SUSEQt5.14/182/
Wouldn't a QDesktopService::openUrl("mailto:/...") suffice?
I looked at KToolInvocation::invokeMailer
Internally it has 3 paths:
A windows path, which builds a URL, finds the URL handler for mailto and manually invokes it with low level windows commands
A linux-non-kmail path which builds a URL, finds the URL handler for mailto and manually invokes it with low level kdeinit commands
A linux-kmail specific path, which builds a command line specifically for kmail.
I feel like we should be able to have the best of both worlds and reduce KToolInvokcation to provide a public API with convenient type-safe API to build a URL which then internally calls QDesktopServices:openUrl()
@hindenburg This phabricator isn't a change in itself.. The relevant commit from your CI is this one: https://invent.kde.org/utilities/konsole/-/merge_requests/194
I don't see how it could have broken the unit tests in itself.
KF6 meeting notes:
The still useful parts of KToolInvocation should be redesigned as jobs on top of ApplicationLauncherJob, like MailClientLauncherJob, TerminalLauncherJob etc. Maybe in KIO for now, then it can all move down together for KF6.
I volunteer to write these jobs, but I'm having second thoughts about providing a replacement in KIOGui for API in KService. It *raises* the amount of needed dependencies.
Solution 1: we find a solution for KRecentDocuments (like forking it in KService or below, under a new name), we fork CommandLauncherJob in KService (no name clash, thanks to the KIO namespace), and we can then have these jobs in KService right away. When I say forking, the old class could become a wrapper for the underlying (new) class, but that's more work [for a job it means using the other as a subjob...].
Solution 2: we (I) just implement these jobs in KIO, and solve the dependency issue at KF6 time.
One concrete problem with solution 1 is that kdesu uses KToolInvocation::kdeinitExecWait and doesn't link to KIO. But well, we can just wait for that one, or maybe use QProcess.
Solution 1: we find a solution for KRecentDocuments (like forking it in KService or below, under a new name), we fork CommandLauncherJob in KService (no name clash, thanks to the KIO namespace), and we can then have these jobs in KService right away. When I say forking, the old class could become a wrapper for the underlying (new) class, but that's more work [for a job it means using the other as a subjob...].
We would also need desktopexecparser and potentially add KWindowSystem dependency for the startup id handling
Solution 2: we (I) just implement these jobs in KIO, and solve the dependency issue at KF6 time.
Seems like the easiest solution to me
Interesting about desktopexecparser. I always assumed it would just move along with CommandLauncherJob/ApplicationLauncherJob/kprocessrunner.
This made me realize that it needs KProtocolInfo::isHelperProtocol(url) / KProtocolInfo::isKnownProtocol(url) which very much belong to KIO,
but that's only in the static method KIO::DesktopExecParser::hasSchemeHandler() which is only used in OpenUrlJob (and KRun). I just added KF6 TODOs to solve that, quite easy: https://invent.kde.org/frameworks/kio/commit/65a374daa83947cbdf8c9d427d1462b4af8ae1af
So yes, it can move to KService too.
For KWindowSystem, I guess it would be ok to add that dependency to KService (tier 3).
But yes, that's a lot of things to fork right now, let's go for solution 2. Thanks for your thoughts on this.