> On Kvě. 7, 2016, 9:40 dop., Dominik Haumann wrote:
> > I can see that the patch works, since Qt's QTextLayout functions are used 
> > for cursor navigation.
> > 
> > I dislike the part that exposes the "currentTextLayout()", since it exposes 
> > more API (but ok, maybe we must), and it is not flexible. For instance, 
> > ktexteditor.git has a multicursor branch, and just getting the "current" 
> > text layout is not enough. Could you change it to textLayout(int line), 
> > along with a comment that the returned pointer is possibly a nullptr?
> > 
> > PS: KateViewInternal.cpp also has a KateWrappingCursor. A proper solution 
> > would be to expose this class so that proper text navigation is available 
> > everywhere (e.g. KTextEditor::ViewCursor, just like 
> > KTextEditor::DocumentCursor).
> 
> Jan Grulich wrote:
>     I'm trying to expose KateWrappingCursor class, but problem is that it 
> requires access to KateViewInternal which we would still need to expose 
> instead of currentTextLayout(). Any idea how to solve that?
> 
> Dominik Haumann wrote:
>     Hm, say we have KateViewCursor with the following constructor:
>     
>         KTextEditor::KateViewCursor(KTextEditor::View * view);
>     
>     Then, can't we qobject_cast this view to KTextEditor::ViewPrivate ? Then 
> we have access to our internal KateView and KateViewInternal.
>     
>     We'd need a pimpl-class, i.e. a d-pointer to store the 
> qobject_cast<KTextEditor::ViewPrivate *>(view) as a member variable.
>     
>     In addition to this constructor, most of the API from 
> KTextEditor::DocumentCursor [1] and what the Wrapping cursor etc can do right 
> now.
>     
>     Does that make sense / is that feasible?
>     
>     [1] 
> http://api.kde.org/frameworks-api/frameworks5-apidocs/ktexteditor/html/classKTextEditor_1_1DocumentCursor.html

It makes sense, there is just a problem again with accessing to 
KateViewInternal from KTextEditor::ViewPrivate. KateViewInternal is stored 
there as a private property, the only way would be to expose it instead of 
currentTextLayout().


- Jan


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


On Kvě. 5, 2016, 10:53 dop., Jan Grulich wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127843/
> -----------------------------------------------------------
> 
> (Updated Kvě. 5, 2016, 10:53 dop.)
> 
> 
> Review request for Kate, KDE Frameworks, Christoph Cullmann, and Dominik 
> Haumann.
> 
> 
> Repository: ktexteditor
> 
> 
> Description
> -------
> 
> When using Indic locales composed characters are not properly removed. 
> Pressing "delete" or "backspace" should remove the entire composed character 
> and not only one base character. You can see how it should behave when using 
> another text editor (e.g. libreoffice)
> 
> Can be tested with these words: ?????????? or ??????????? or ????????
> 
> Technical details:
> I'm not really sure whether exporting current text layout is the right way to 
> do this, I found that this is used when calling moveChar() which moves the 
> cursor exactly by one composed character so I was trying to find a way to do 
> it similarly.
> 
> 
> Diffs
> -----
> 
>   src/document/katedocument.cpp 73778a1 
>   src/view/kateview.h 08db0df 
>   src/view/kateview.cpp fda6b2d 
> 
> Diff: https://git.reviewboard.kde.org/r/127843/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jan Grulich
> 
>

_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Reply via email to