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
