Differential D7660 Diff 24338 src/document/katedocument.cpp

Changeset View

Standalone View

View Options

src/document/katedocument.cpp

Show First 20 Lines • Show All 3160 Lines • ▼ Show 20 Line(s)3150{
3161 3161​
3162 uint col = qMax(c.column(), 0); 3162​ uint col = qMax(c.column(), 0);
3163 uint line = qMax(c.line(), 0); 3163​ uint line = qMax(c.line(), 0);
3164 3164​
3165 if ((col == 0) && (line == 0)) { 3165​ if ((col == 0) && (line == 0)) {
3166 return; 3166​ return;
3167 } 3167​ }
3168 3168​
3169 if (col > 0) { 3169​ if (col > 0) {
3170 if (!(config()->backspaceIndents())) { 3170​ if (config()->backspaceIndents()) {
dhaumannUnsubmitted
Not Done

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()));
}
dhaumann: I would prefer to have this if() only once: KTextEditor::Cursor beginCursor(line…
3171 // ordinary backspace ​
3172 KTextEditor::Cursor beginCursor(line, view->textLayout(c)->previousCursorPosition(c.column())); ​
3173 KTextEditor::Cursor endCursor(line, col); ​
3174 ​
3175 removeText(KTextEditor::Range(beginCursor, endCursor)); ​
3176 // in most cases cursor is moved by removeText, but we should do it manually ​
3177 // for past-end-of-line cursors in block mode ​
3178 view->setCursorPosition(beginCursor); ​
3179 } else { ​
3180 // backspace indents: erase to next indent position 3171​ // backspace indents: erase to next indent position
3181 Kate::TextLine textLine = m_buffer->plainLine(line); 3172​ Kate::TextLine textLine = m_buffer->plainLine(line);
3182 3173​
3183 // don't forget this check!!!! really!!!! 3174​ // don't forget this check!!!! really!!!!
3184 if (!textLine) { 3175​ if (!textLine) {
3185 return; 3176​ return;
3186 } 3177​ }
3187 3178​
3188 int colX = textLine->toVirtualColumn(col, config()->tabWidth()); 3179​ int colX = textLine->toVirtualColumn(col, config()->tabWidth());
3189 int pos = textLine->firstChar(); 3180​ int pos = textLine->firstChar();
3190 if (pos > 0) { 3181​ if (pos > 0) {
3191 pos = textLine->toVirtualColumn(pos, config()->tabWidth()); 3182​ pos = textLine->toVirtualColumn(pos, config()->tabWidth());
3192 } 3183​ }
3193 3184​
3194 if (pos < 0 || pos >= (int)colX) { 3185​ if (pos < 0 || pos >= (int)colX) {
3195 // only spaces on left side of cursor 3186​ // only spaces on left side of cursor
3196 indent(KTextEditor::Range(line, 0, line, 0), -1); 3187​ indent(KTextEditor::Range(line, 0, line, 0), -1);
3197 } else { 3188​ }
3198 KTextEditor::Cursor beginCursor(line, view->textLayout(c)->previousCursorPosition(c.column())); 3189​ }
3190​ if (!config()->backspaceIndents() || pos) {
brauchUnsubmitted
Not Done

pos is not defined at this place

brauch: pos is not defined at this place
3191​ KTextEditor::Cursor beginCursor(line, 0);
3199 KTextEditor::Cursor endCursor(line, col); 3192​ KTextEditor::Cursor endCursor(line, col);
3200 3193​ if (!view->config()->backspaceRemoveComposed()) { // Normal backspace behavior
3194​ // move to left of surrogate pair
3195​ if (!isValidTextPosition(beginCursor)) {
3196​ Q_ASSERT(col >= 2);
3197​ beginCursor.setColumn(col - 2);
3198​ }
3199​ beginCursor.setColumn(col - 1);
3200​ } else {
3201​ beginCursor.setColumn(view->textLayout(c)->previousCursorPosition(c.column()));
dhaumannUnsubmitted
Not Done

Same here: Please if() case only once, which leads to less convoluted code.

dhaumann: Same here: Please if() case only once, which leads to less convoluted code.
3202​ }
3201 removeText(KTextEditor::Range(beginCursor, endCursor)); 3203​ removeText(KTextEditor::Range(beginCursor, endCursor));
3202 // in most cases cursor is moved by removeText, but we should do it manually 3204​ // in most cases cursor is moved by removeText, but we should do it manually
3203 // for past-end-of-line cursors in block mode 3205​ // for past-end-of-line cursors in block mode
3204 view->setCursorPosition(beginCursor); 3206​ view->setCursorPosition(beginCursor);
3205 } 3207​ }
anthonyfieroniUnsubmitted
Not Done

This peace of code should be simplified, to not have code dublication.

if (config()->backspaceIndents()) {
     // backspace indents: erase to next indent position
      Kate::TextLine textLine = m_buffer->plainLine(line);

      // don't forget this check!!!! really!!!!
      if (!textLine) {
          return;
      }

      int colX = textLine->toVirtualColumn(col, config()->tabWidth());
      int pos = textLine->firstChar();
      if (pos > 0) {
          pos = textLine->toVirtualColumn(pos, config()->tabWidth());
      }

      if (pos < 0 || pos >= (int)colX) {
          // only spaces on left side of cursor
          indent(KTextEditor::Range(line, 0, line, 0), -1);
      }
}

Then validate pos and add 2 backspace behaviors.

anthonyfieroni: This peace of code should be simplified, to not have code dublication. ``` if (config()…
3206 } ​
3207 } else { 3208​ } else {
3208 // col == 0: wrap to previous line 3209​ // col == 0: wrap to previous line
3209 if (line >= 1) { 3210​ if (line >= 1) {
3210 Kate::TextLine textLine = m_buffer->plainLine(line - 1); 3211​ Kate::TextLine textLine = m_buffer->plainLine(line - 1);
3211 3212​
3212 // don't forget this check!!!! really!!!! 3213​ // don't forget this check!!!! really!!!!
3213 if (!textLine) { 3214​ if (!textLine) {
3214 return; 3215​ return;
▲ Show 20 Lines • Show All 2793 Lines • Show Last 20 Lines