-----------------------------------------------------------
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
> 
>

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

Reply via email to