Restore KTextEditor Document Dbus bindings
AbandonedPublic

Authored by cullmann on Feb 16 2018, 4:18 PM.

Details

Summary

Enable access to the document itself via dbus; this is mostly a revert of c8ce78a02794193f20caacdfb5ef68ac0d26064d and 6ff7cedb57abe7deb37e33bfa82d2f5530eb295c in the kate git repo (which removed the dbus bindings with no explanation other than "cleanups")

Intends to fix https://bugs.kde.org/show_bug.cgi?id=369623

Test Plan

qdbus org.kde.kate-6503 /Kate/Document/2 text
qdbus org.kde.kate-6503 /Kate/Document/2 file

Diff Detail

Repository
R39 KTextEditor
Lint
Lint Skipped
Unit
Unit Tests Skipped
lukedashjr created this revision.Feb 16 2018, 4:18 PM
Restricted Application added a subscriber: Frameworks. · View Herald TranscriptFeb 16 2018, 4:18 PM
lukedashjr requested review of this revision.Feb 16 2018, 4:18 PM

You can try to add DBus in Kate (application) rather than KTextEditor (framework).

You can try to add DBus in Kate (application) rather than KTextEditor (framework).

Won't that break compatibility? (and fail to cover KWrite or other similar applications?)

It will not break compatibility or you should supposed how? Kate and KWrite present in same repo, so the code can be shared, but yes it will not have this feature in other apps that uses KTextEditor. But you can read @cullmann wrote in bug report that it doesn't want KTextEditor to depend on QDBus.

It will not break compatibility or you should supposed how? Kate and KWrite present in same repo, so the code can be shared, but yes it will not have this feature in other apps that uses KTextEditor. But you can read @cullmann wrote in bug report that it doesn't want KTextEditor to depend on QDBus.

I don't think I would know how to make these changes. I was only able to get this to work because it was mostly a simple revert.

anthonyfieroni added a comment.EditedFeb 16 2018, 10:37 PM

I don't think I would know how to make these changes. I was only able to get this to work because it was mostly a simple revert.

It should be same: at https://phabricator.kde.org/source/kate/browse/master/kate/katedocmanager.cpp;2641fc835e5fecb539122dcfbee1caa42617ebe4$89

qRegisterMetaType<KTextEditor::Document*>("KTextEditor::Document*");
new DocumentAdaptor(doc);
const QString pathName = QString::fromLatin1("/Kate/Document/%1").arg(++dbusDocumentNumber);
QDBusConnection::sessionBus().registerObject(pathName, doc, QDBusConnection::ExportAdaptors | QDBusConnection::ExportScriptableSlots);

And all others. You can test it or adapt it?

cullmann requested changes to this revision.Jul 14 2018, 8:25 PM

I still don't agree with having that just for the use-case of "auto-save" untitled documents.

I would make much more sense to have a more clever handling for that, which would solve bug https://bugs.kde.org/show_bug.cgi?id=394833

I am fine with a solution for that, but not for introducing again some interface that has only one use and leads to overhead registering stuff at dbus for each open document.

This revision now requires changes to proceed.Jul 14 2018, 8:25 PM
Restricted Application edited subscribers, added: kde-frameworks-devel, kwrite-devel; removed: Frameworks. · View Herald TranscriptJul 14 2018, 8:25 PM
cullmann commandeered this revision.Aug 14 2018, 1:22 PM
cullmann edited reviewers, added: lukedashjr; removed: cullmann.

As said, I agree a fix for the unsaved documents vs. no swap files problem is wanted, but a single use case is not enough to bring back the dbus overhead per document.

cullmann abandoned this revision.Aug 14 2018, 1:22 PM