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

[Milian Wolff]

mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.


  now that this API becomes public, it must be improved to make it better understandable to the public
  
  and note that you cannot add new virtual methods to this interface, as it would break the ABI: https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B
  You will have to create a separate interface for this, similar to what we have done in the past, with a TODO note that this should be merged in time for KF6.
  
  I'm very much interested in your app btw - will you support embedding it into e.g. KDevelop as a patch viewer? Kompare is pretty much unmaintained, and I like what I see in your screenshot!

INLINE COMMENTS

> view.h:464
> +     */
> +    virtual void scrollPos(KTextEditor::Cursor &c) = 0;
> +

should be scrollToPos, c -> cursor, and make the cursor const

> view.h:471
> +     */
> +    virtual void scrollColumns(int x) = 0;
> +

scrollToColumn, x -> column

> view.h:479
> +     */
> +    virtual KTextEditor::Cursor maxStartPos() const = 0;
> +

why is this not the document end pos? because of virtual cursors? if so, please rename this to `viewEndCursor()` or similar, I don't see why this should be called `startPos`

> view.h:487
> +     */
> +    virtual int startLine() const = 0;
> +

firstVisibleLine

> view.h:495
> +     */
> +    virtual int endLine() const = 0;
> +

lastVisibleLine

> view.h:502
> +     */
> +    virtual QRect textareaRect() const = 0;
>      /*

textAreaRect

> kateview.cpp:2557
> +
> +void KTextEditor::ViewPrivate::scrollColumns(int x) {
> +    m_viewInternal->scrollColumns(x);

here and below: put the { on its own line, like you did above

> kateview.cpp:2575
> +QRect KTextEditor::ViewPrivate::textareaRect() const {
> +    QRect rect = QRect(m_viewInternal->mapTo(this, m_viewInternal->rect().topLeft()), m_viewInternal->mapTo(this, m_viewInternal->rect().bottomRight()));
> +

const auto sourceRect = m_viewInternal->rect();
  const auto topLeft = m_viewInternal->mapTo(this, sourceRect.topLeft());
  const auto bottomRight = m_viewInternal->mapTo(this, sourceRect.bottomRight());
  return {topLeft, bottomRight};

REPOSITORY
  R39 KTextEditor

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

To: jsalatas, #ktexteditor, #frameworks, mwolff
Cc: mwolff, kwrite-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170305/43233fa1/attachment.html>