D10587: Restore KTextEditor Document Dbus bindings

[Christoph Cullmann]

cullmann commandeered this revision.
cullmann edited reviewers, added: lukedashjr; removed: cullmann.
cullmann added a comment.


  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.

REPOSITORY
  R39 KTextEditor

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

To: cullmann, dhaumann, gregormi, lukedashjr
Cc: kwrite-devel, kde-frameworks-devel, anthonyfieroni, #ktexteditor, #kate, 
asturmlechner, gregormi, dhaumann, cullmann, michaelh, kevinapavew, ngraham, 
bruns, demsking, head7, kfunk, sars

D10587: Restore KTextEditor Document Dbus bindings

[Christoph Cullmann]

cullmann abandoned this revision.

REPOSITORY
  R39 KTextEditor

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

To: cullmann, dhaumann, gregormi, lukedashjr
Cc: kwrite-devel, kde-frameworks-devel, anthonyfieroni, #ktexteditor, #kate, 
asturmlechner, gregormi, dhaumann, cullmann, michaelh, kevinapavew, ngraham, 
bruns, demsking, head7, kfunk, sars

D10587: Restore KTextEditor Document Dbus bindings

[Christoph Cullmann]

cullmann requested changes to this revision.
cullmann added a comment.
This revision now requires changes to proceed.
Restricted Application edited subscribers, added: kde-frameworks-devel, 
kwrite-devel; removed: Frameworks.


  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.

REPOSITORY
  R39 KTextEditor

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

To: lukedashjr, cullmann, dhaumann, gregormi
Cc: kwrite-devel, kde-frameworks-devel, anthonyfieroni, #ktexteditor, #kate, 
asturmlechner, gregormi, dhaumann, cullmann, michaelh, kevinapavew, ngraham, 
bruns, demsking, head7, kfunk, sars, #frameworks

D10587: Restore KTextEditor Document Dbus bindings

[Anthony Fieroni]

anthonyfieroni added a comment.


  In D10587#207885 , @lukedashjr 
wrote:
  
  > 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*");
new DocumentAdaptor(doc);
const QString pathName = 
QString::fromLatin1("/Kate/Document/%1").arg(++dbusDocumentNumber);
QDBusConnection::sessionBus().registerObject(pathName, doc, 
QDBusConnection::ExportAdaptors | QDBusConnection::ExportScriptableSlots);
  
  Other is same.

REPOSITORY
  R39 KTextEditor

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

To: lukedashjr, cullmann, dhaumann, gregormi
Cc: anthonyfieroni, #ktexteditor, #kate, asturmlechner, gregormi, dhaumann, 
cullmann, #frameworks, michaelh, kevinapavew, ngraham, demsking, head7, kfunk, 
sars

D10587: Restore KTextEditor Document Dbus bindings

[Andreas Sturmlechner]

asturmlechner added a comment.


  This change has history, for reference: 
https://git.reviewboard.kde.org/r/129398/

REPOSITORY
  R39 KTextEditor

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

To: lukedashjr, cullmann, dhaumann, gregormi
Cc: anthonyfieroni, #ktexteditor, #kate, asturmlechner, gregormi, dhaumann, 
cullmann, #frameworks, michaelh, kevinapavew, ngraham, demsking, head7, kfunk, 
sars

D10587: Restore KTextEditor Document Dbus bindings

[Luke Dashjr]

lukedashjr added a comment.


  In D10587#207874 , @anthonyfieroni 
wrote:
  
  > 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.

REPOSITORY
  R39 KTextEditor

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

To: lukedashjr, cullmann, dhaumann, gregormi
Cc: anthonyfieroni, #ktexteditor, #kate, asturmlechner, gregormi, dhaumann, 
cullmann, #frameworks, michaelh, kevinapavew, ngraham, demsking, head7, kfunk, 
sars

D10587: Restore KTextEditor Document Dbus bindings

[Anthony Fieroni]

anthonyfieroni added a comment.


  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.

REPOSITORY
  R39 KTextEditor

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

To: lukedashjr, cullmann, dhaumann, gregormi
Cc: anthonyfieroni, #ktexteditor, #kate, asturmlechner, gregormi, dhaumann, 
cullmann, #frameworks, michaelh, kevinapavew, ngraham, demsking, head7, kfunk, 
sars

D10587: Restore KTextEditor Document Dbus bindings

[Luke Dashjr]

lukedashjr added a comment.


  In D10587#207857 , @anthonyfieroni 
wrote:
  
  > 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?)

REPOSITORY
  R39 KTextEditor

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

To: lukedashjr, cullmann, dhaumann, gregormi
Cc: anthonyfieroni, #ktexteditor, #kate, asturmlechner, gregormi, dhaumann, 
cullmann, #frameworks, michaelh, kevinapavew, ngraham, demsking, head7, kfunk, 
sars

D10587: Restore KTextEditor Document Dbus bindings

[Anthony Fieroni]

anthonyfieroni added a comment.


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

REPOSITORY
  R39 KTextEditor

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

To: lukedashjr, cullmann, dhaumann, gregormi
Cc: anthonyfieroni, #ktexteditor, #kate, asturmlechner, gregormi, dhaumann, 
cullmann, #frameworks, michaelh, kevinapavew, ngraham, demsking, head7, kfunk, 
sars

D10587: Restore KTextEditor Document Dbus bindings

[Luke Dashjr]

lukedashjr created this revision.
lukedashjr added reviewers: cullmann, dhaumann, gregormi.
lukedashjr added projects: KTextEditor, Kate, Frameworks.
Restricted Application added a subscriber: Frameworks.
lukedashjr requested review of this revision.

REVISION 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

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/CMakeLists.txt
  src/document/katedocument.cpp
  src/utils/document.cpp
  src/utils/documentadaptor_p.h

To: lukedashjr, cullmann, dhaumann, gregormi
Cc: #ktexteditor, #kate, asturmlechner, gregormi, dhaumann, cullmann, 
#frameworks, michaelh, kevinapavew, ngraham, demsking, head7, kfunk, sars