kioexec: delegate upload to a kded module
ClosedPublic
Actions

Authored by elvisangelaccio on Mar 12 2017, 4:38 PM.

Details

Reviewers

Commits

R241:a7758fb66481: kioexec: delegate upload to a kded module

Summary

This introduces a kded module that kioexec can use to watch the cached
files for changes. This allows kioexec to be fully functional even with
applications that fork on startup, like libreoffice.

If the kded module is up and running, kioexec tells it (via dbus)
which file should be watched for changes and where to upload it when
it is actually changed. All the files watched by the module are deleted
when the module is destroyed.

As a bonus side effect, the dialog that asks if changes should be uploaded
is now displayed every time the user saves the file.

If dbus is not available or the kded module is otherwise disabled,
then kioexec falls back on the old behavior.

BUG: 252026
BUG: 370532
FIXED-IN: 5.34

Diff Detail

Repository

R241 KIO

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

elvisangelaccio created this revision.Mar 12 2017, 4:38 PM

Restricted Application added a project: Frameworks. · View Herald TranscriptMar 12 2017, 4:38 PM

Restricted Application added a subscriber: Frameworks. · View Herald Transcript

elvisangelaccio edited the summary of this revision. (Show Details)Mar 12 2017, 4:52 PM

Nice idea.

Can you use kiod rather than kded? Same plugin mechanism exactly but this way no dependency on kded.

dfaure requested changes to this revision.Mar 27 2017, 8:02 PM

dfaure added inline comments.

src/kioexec/kioexecd.cpp
75	QUrl(destUrl) would be enough, you use toString() in the caller.
94	Nested event loop! In a multi-purpose daemon, very bad idea. Been there... Just connect the job to a lambda.
src/kioexec/main.cpp
123	Using a qdbusxml2cpp-generated header would provide more compile-time checking.

This revision now requires changes to proceed.Mar 27 2017, 8:02 PM

In D5030#98126, @dfaure wrote:

Nice idea.

Can you use kiod rather than kded? Same plugin mechanism exactly but this way no dependency on kded.

How do I do that? Do I just install the plugin to $plugindir/kf5/kiod rather than $plugindir/kf5/kded?

Yes, that should be it. And for good practice, rather than hardcoding kded5 or kiod5 in the calling code, create a dbus .service file to autostart it (two purposes: making it independent from whoever is hosting the service, and starting kiod if it's not already running). See kio/src/kpasswdserver for an example.

Ok I got it working, thanks. There is a downside though, it seems the module is never deleted, not even when kiod5 quits. Bug in kiod? It was deleted as expected with kded, instead.

Addressed all David's issues.

Good point, I added a destructor to KIOD now, please try again ;)

src/kioexec/kioexecd.cpp
55	btw iterating over keys() is bad practice (slow because it requires creating and filling a temporary container first). Better use a proper QMap iterator.

Tested against latest master, now the module is properly cleaned up upon kiod destruction.
Use iterators for the map

Feel free to push after that last fix.

src/kioexec/kioexecd.cpp
59	QFile::remove takes a QString, this call to QFile::encodeName() is wrong and unnecessary.

This revision is now accepted and ready to land.Apr 15 2017, 8:14 AM

Closed by commit R241:a7758fb66481: kioexec: delegate upload to a kded module (authored by elvisangelaccio). · Explain WhyApr 15 2017, 3:27 PM

This revision was automatically updated to reflect the committed changes.

elvisangelaccio marked an inline comment as done.

elvisangelaccio mentioned this in R241:a7758fb66481: kioexec: delegate upload to a kded module.

Revision Contents
Changeset List

		Path
M		src/kioexec/CMakeLists.txt (28 lines)
A	M	src/kioexec/kioexecd.h (53 lines)
A	M	src/kioexec/kioexecd.cpp (111 lines)
A	M	src/kioexec/kioexecd.json (11 lines)
M		src/kioexec/main.h (1 line)
M		src/kioexec/main.cpp (13 lines)
A	M	src/kioexec/org.kde.kioexecd.service.in (3 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	12414	8ea3b34		Mar 12 2017, 4:38 PM	★	★
Diff 2	13163			Apr 6 2017, 6:41 PM	★	★
Diff 3	13202			Apr 7 2017, 4:13 PM	★	★
Diff 4	13471	e011b90	R241:a7758fb66481f05d1791687b415331a4f4af782f	Apr 15 2017, 3:25 PM	★	★