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

Reply via email to