[Differential] [Request, 6 lines] D4538: [KTextEditor] consistent conversion from/to cursor to/from coordinates

[John Salatas]

jsalatas created this revision.
jsalatas added reviewers: Frameworks, Plasma.
jsalatas set the repository for this revision to R39 KTextEditor.
Restricted Application added projects: Plasma, Frameworks.
Restricted Application added subscribers: kwrite-devel, plasma-devel.

REVISION SUMMARY
  This patch addresses 3 issues
  
  1. Currently if you try to convert the cursor to coordinates and then convert these coordinates back to a cursor again, it produces an invalid cursor (-1, -1) when the original cursor is located at the end of line
  
  2. Converting back to cursor coordinates that have been produced by `cursorToCoordinate(view->cursorPosition())` and `coordinatesToCursor(view->cursorPositionCoordinates())` doesn't always produce the same results as the `includeBorder` option doesn't seem to be used consistently.
  
  3. Although I cannot find a test case for this, it doen't seem to make any sense the line `scrollPos(max, max.column(), calledExternally);` in `KateViewInternal::makeVisible` function.   Was it supposed to be that way (ie force when the last line is not empty and not force when it is), or is it just a typo? :\  I would appreciate any reasoning about this!

TEST PLAN
  1. Create a new app with a single `KTextEditor::View` widget (similar to kwrite).
  
  2. connect the view's `cursorPositionChanged` signal with a slot in the window which contains the following code
  
    qDebug() << " original cursor" << view->cursorPosition();
    QPoint p1 = view->cursorToCoordinate(view->cursorPosition());
    KTextEditor::Cursor c1 = view->coordinatesToCursor(p1);
    qDebug() << "converted cursor" << c1;
    qDebug() << "converted cursor 2" << view->coordinatesToCursor(view->cursorPositionCoordinates());
  
  
  
  3. Run the application and start typing (I typed 52 characters in the first line) and then pressed the <Enter> key sometimes)
  
  4. Press Back Arrow key until you are in the beginning of the document.
  
  In steps 3 and 4 the three cursors are the same (this wasn't the case before).
  
  Before you get something like the following
  
    original cursor (0, 1)
    converted cursor (-1, -1)
    converted cursor 2 (-1, -1)
     original cursor (0, 2)
    converted cursor (-1, -1)
    converted cursor 2 (-1, -1)
     original cursor (0, 3)
    converted cursor (-1, -1)
    converted cursor 2 (-1, -1)
     original cursor (0, 4)
    converted cursor (-1, -1)
    converted cursor 2 (-1, -1)
     original cursor (0, 5)
    converted cursor (-1, -1)
    converted cursor 2 (-1, -1)
     original cursor (0, 6)
    converted cursor (-1, -1)
    converted cursor 2 (-1, -1)
     original cursor (0, 7)
    converted cursor (-1, -1)
    ......
     original cursor (0, 48)
    converted cursor (-1, -1)
    converted cursor 2 (-1, -1)
     original cursor (0, 49)
    converted cursor (-1, -1)
    converted cursor 2 (-1, -1)
     original cursor (0, 50)
    converted cursor (-1, -1)
    converted cursor 2 (-1, -1)
     original cursor (0, 51)
    converted cursor (-1, -1)
    converted cursor 2 (-1, -1)
     original cursor (0, 52)
    converted cursor (-1, -1)
    converted cursor 2 (-1, -1)
     original cursor (1, 0)
    converted cursor (-1, -1)
    converted cursor 2 (-1, -1)
     original cursor (2, 0)
    converted cursor (-1, -1)
    converted cursor 2 (-1, -1)
     original cursor (3, 0)
    converted cursor (-1, -1)
    converted cursor 2 (-1, -1)
    .....
     original cursor (0, 52)
    converted cursor (-1, -1)
    converted cursor 2 (-1, -1)
     original cursor (0, 51)
    converted cursor (0, 51)
    converted cursor 2 (-1, -1)
     original cursor (0, 50)
    converted cursor (0, 50)
    converted cursor 2 (-1, -1)
     original cursor (0, 49)
    converted cursor (0, 49)
    converted cursor 2 (-1, -1)
     original cursor (0, 48)
    converted cursor (0, 48)
    converted cursor 2 (-1, -1)
     original cursor (0, 47)
    converted cursor (0, 47)
    converted cursor 2 (0, 51)
     original cursor (0, 46)
    converted cursor (0, 46)
    converted cursor 2 (0, 50)
     original cursor (0, 45)
    converted cursor (0, 45)
    converted cursor 2 (0, 49)
     original cursor (0, 44)
    converted cursor (0, 44)
    converted cursor 2 (0, 48)
     original cursor (0, 43)
    converted cursor (0, 43)
    converted cursor 2 (0, 47)
     original cursor (0, 42)
    converted cursor (0, 42)
    converted cursor 2 (0, 46)
     original cursor (0, 41)
    converted cursor (0, 41)
    converted cursor 2 (0, 45)
    .....
     original cursor (0, 6)
    converted cursor (0, 6)
    converted cursor 2 (0, 10)
     original cursor (0, 5)
    converted cursor (0, 5)
    converted cursor 2 (0, 9)
     original cursor (0, 4)
    converted cursor (0, 4)
    converted cursor 2 (0, 8)
     original cursor (0, 3)
    converted cursor (0, 3)
    converted cursor 2 (0, 7)
     original cursor (0, 2)
    converted cursor (0, 2)
    converted cursor 2 (0, 6)
     original cursor (0, 1)
    converted cursor (0, 1)
    converted cursor 2 (0, 5)
     original cursor (0, 0)
    converted cursor (0, 0)
    converted cursor 2 (0, 4)
  
  after this patch is applied the converted cursor are consistent with the original (correct) cursor.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D4538

AFFECTED FILES
  src/view/kateview.cpp
  src/view/kateviewinternal.cpp

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: jsalatas, #frameworks, #plasma
Cc: plasma-devel, kwrite-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170209/4aed2258/attachment-0001.html>