jsalatas updated this revision to Diff 11494.
jsalatas added a comment.

  1. I abandoned the 3rd issue I mention in the summary about the line 
`scrollPos(max, max.column(), calledExternally);` as I could neither verify nor 
disprove if this was intended or not.
  
  2. In `cursorToCoordinate()` I added an extra check to see if we are behind 
end of line and return and invalid point (-1, -1):
  
    if (cursor.column() > layout.lineLayout().textLength()) {
        return QPoint(-1, -1);
    }
  
  as according to QT's documentation 
(http://doc.qt.io/qt-5/qtextline.html#cursorToX)
  
  > If cursorPos is not a valid cursor position, the nearest valid cursor 
position will be used instead, and cursorPos will be modified to point to this 
valid cursor position.
  
  and without this extra check, the "behind end of line should give an invalid 
cursor" tests in kateview_test.cpp would fail.
  
  3. I also added the relevant tests to kateview_test.cpp, as per cullmann's 
feedback.
  
  4. KDevelop seems to work.
  
  I have checked it and I didn't see any issues to the Navigation Widget popup 
tooltips which indeed seems to extensively use coversions between cursors and 
coordinates, as mentioned by brauch. Furthermore:
  
  - cursorPositionCoordinates isn't used in kdevelop and kdevplatform, so the 
2nd issue described in the summary shouldn't affect kdevelop and kdevplatform 
in any way.
  - 1st issue in the summary is related to cursors/positions at the end of 
line, so my intuition suggests that it wouldn't be related to any context 
related to kdevelop. :)

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4538?vs=11137&id=11494

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

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

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

To: jsalatas, #frameworks, #plasma, #ktexteditor
Cc: brauch, cullmann, plasma-devel, kwrite-devel, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

Reply via email to