Review Request 127843: Properly remove composed characters

[Dominik Haumann]

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



Ok, I think one more revision and we're good to go. Thanks for working on this!


autotests/src/katedocument_test.cpp (line 431)
<https://git.reviewboard.kde.org/r/127843/#comment64842>

    Is QString::fromUtf8() better here?



autotests/src/katedocument_test.cpp (line 434)
<https://git.reviewboard.kde.org/r/127843/#comment64843>

    QString::fromUtf8() ?



autotests/src/katedocument_test.cpp (line 438)
<https://git.reviewboard.kde.org/r/127843/#comment64844>

    QString::fromUtf8()?



src/view/kateview.h (line 396)
<https://git.reviewboard.kde.org/r/127843/#comment64846>

    Could we change this to the more generic way (line + cursor overload):
    
        QTextLayout * textLayout(int line) const;
        QTextLayout * textLayout(const KTextEditor::Cursor & pos) const;
    
    I have the feeling that we will need it anyways for the multicursor branch.



src/view/kateview.cpp (line 2589)
<https://git.reviewboard.kde.org/r/127843/#comment64845>

    This also works:
    
        KateLineLayoutPtr thisLine = m_viewInternal->cache()->line(cursorPosition());
    
    If we request the cache for an invalid line, the returned KateLineLayoutPtr is invalid, and then the ->layout() call will access a nullptr.
    
    Please guard against this and return nullptr, in case the thisLine is invalid.


- Dominik Haumann


On May 26, 2016, 10:06 a.m., Jan Grulich wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127843/
> -----------------------------------------------------------
> 
> (Updated May 26, 2016, 10:06 a.m.)
> 
> 
> 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
> -----
> 
>   autotests/src/katedocument_test.h 8abbad9 
>   autotests/src/katedocument_test.cpp dd41e0f 
>   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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160526/e2ce48f0/attachment.html>