D4947: [KTextEditor] Expose additional internal View's functionality to the public API

[John Salatas]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170307/f68d734b/attachment-0001.html>