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