Review Request 129398: Restore KTextEditor Document Dbus bindings

[Luke Dashjr]

> On Nov. 17, 2016, 9:18 p.m., Dominik Haumann wrote:
> > src/utils/document.cpp, line 97
> > <https://git.reviewboard.kde.org/r/129398/diff/1/?file=485441#file485441line97>
> >
> >     Is this correct? y and x are switched, do you know any reason for this? Did you test?

I just copied the vast majority of this code from the older KTextEditor. I only tested 'text' and 'name', but I see no reason the rest shouldn't continue to function like it used to.

Cursor seems to indeed construct with (line, column) which is the equivalent of QPoint's (y, x), so I would think this is correct. Obviously I can't really test whether it is correct, because I do not know for certain that the intended input is the input used, but it seems reasonable. (I also have no idea how to use a QPoint in dbus)


> On Nov. 17, 2016, 9:18 p.m., Dominik Haumann wrote:
> > src/utils/documentadaptor_p.h, line 26
> > <https://git.reviewboard.kde.org/r/129398/diff/1/?file=485442#file485442line26>
> >
> >     Given this is private API, I would prefer to not have this in namespace KTextEditor, and instead name this class either "KateDocumentAdaptor", or put into namespace "Kate" with current name.
> >     
> >     And please reindent / format code similar to other comment :-)

Wouldn't that break compatibility with the existing (KDE 4) dbus interfaces?


- Luke


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129398/#review100918
-----------------------------------------------------------


On Nov. 18, 2016, 8:53 a.m., Luke Dashjr wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129398/
> -----------------------------------------------------------
> 
> (Updated Nov. 18, 2016, 8:53 a.m.)
> 
> 
> Review request for Kate, KDE Frameworks and Christoph Cullmann.
> 
> 
> Bugs: 369623
>     https://bugs.kde.org/show_bug.cgi?id=369623
> 
> 
> Repository: ktexteditor
> 
> 
> Description
> -------
> 
> 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
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 54cc157 
>   src/document/katedocument.cpp eb30049 
>   src/utils/document.cpp 44dc7cf 
>   src/utils/documentadaptor_p.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/129398/diff/
> 
> 
> Testing
> -------
> 
> qdbus org.kde.kate-6503 /Kate/Document/2 text
> qdbus org.kde.kate-6503 /Kate/Document/2 file
> 
> 
> Thanks,
> 
> Luke Dashjr
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20161118/57dc616b/attachment-0001.html>