jsalatas updated this revision to Diff 12253. jsalatas added a comment.
Diff updated. Hope I addressed all issues :\ See some additional comments/clarifications below: In https://phabricator.kde.org/D4947#92889, @mwolff wrote: > should be scrollToPos, c -> cursor, and make the cursor const No! the internal view's scrollPos (line 180 in src/view/kateview.cpp) modifies the cursor if it is out of scrolling limits. In https://phabricator.kde.org/D4947#93014, @dhaumann wrote: > And should it not be const KTextEditor::Cursor &? No! See my comment above to a similar question asked by @mwolff. In https://phabricator.kde.org/D4947#93173, @tfry wrote: > Thus, I would suggest > a) replacing > > int startLine() const Q_DECL_OVERRIDE; > > with > > KTextEditor::Cursor currentScrollPosition() const Q_DECL_OVERRIDE; > > for more flexibility. > b) same probably for endLine(), although I'm not quite clear on the use case for this one. > c) //either// add scrollToLine(), or remove scrollToColumn() (which could be written as > > scrollToPosition(KTextEditor::Cursor(x, currentScrollPosition.line()) > > , where needed). @tfry the "problem" here that horizontal scrolling is based on pixels rather than cursors, which is not the case with vertical scrolling. REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D4947?vs=12198&id=12253 REVISION DETAIL https://phabricator.kde.org/D4947 AFFECTED FILES src/CMakeLists.txt src/include/CMakeLists.txt src/include/ktexteditor/extensionviewinterface.h src/utils/extensionviewinterface.cpp src/view/kateview.cpp src/view/kateview.h To: jsalatas, #frameworks, tfry, mwolff, #ktexteditor Cc: dhaumann, anthonyfieroni, mwolff, kwrite-devel
