D7660: Fix a regression caused by changing backspace key behavior
Dominik Haumann
noreply at phabricator.kde.org
Mon Sep 18 16:35:58 UTC 2017
dhaumann requested changes to this revision.
dhaumann added a comment.
This revision now requires changes to proceed.
I think one further revision would be helpful, then the patch should be good to go in.
INLINE COMMENTS
> navigationconfigwidget.ui:138
> + <property name="whatsThis">
> + <string>When selected, composed characters are removed with thier diacritics instead of only removing the base character. This is useful for Indic locales.</string>
> + </property>
Typo: their instead of thier.
> navigationconfigwidget.ui:141
> + <property name="text">
> + <string>Backspace key removes character’s base with it’s diacritics</string>
> + </property>
Typo: its instead of it's
> katedocument.cpp:3172
> + KTextEditor::Cursor beginCursor(line, 0);
> + if (!view->config()->backspaceRemoveComposed()) { // Normal backspace behavior
> + beginCursor.setColumn(col - 1);
I would prefer to have this if() only once:
KTextEditor::Cursor beginCursor(line, 0);
KTextEditor::Cursor endCursor(line, col);
if (!view->config()->backspaceRemoveComposed()) { // Normal backspace behavior
beginCursor.setColumn(col - 1);
// move to left of surrogate pair
if (!isValidTextPosition(beginCursor)) {
Q_ASSERT(col >= 2);
beginCursor.setColumn(col - 2);
}
} else {
beginCursor.setColumn(view->textLayout(c)->previousCursorPosition(c.column()));
}
> katedocument.cpp:3208
> } else {
> - KTextEditor::Cursor beginCursor(line, view->textLayout(c)->previousCursorPosition(c.column()));
> + KTextEditor::Cursor beginCursor(line, 0);
> + if (!view->config()->backspaceRemoveComposed()) { // Normal backspace behavior
Same here: Please if() case only once, which leads to less convoluted code.
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D7660
To: safaalfulaij, #ktexteditor, jgrulich, hein, dhaumann
Cc: jgrulich, dhaumann, hein, kwrite-devel, #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170918/d5cbace4/attachment-0001.html>
More information about the Kde-frameworks-devel
mailing list