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