D14826: inline note interface wip #2

[Dominik Haumann]

dhaumann added a comment.


  I think one more iteration, and this can be merged. Can you look into this again?

INLINE COMMENTS

> CMakeLists.txt:5
>    AnnotationInterface CodeCompletionModelControllerInterface MovingCursor Range TextHintInterface
> -  Cursor MarkInterface MovingInterface
> +  Cursor MarkInterface MovingInterface InlineNoteInterface
>    Document  MovingRange View

Could we have also InlineNoteProvider and InlineNote?

> inlinenoteinterface.h:4
> +   This library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Library General Public
> +   License as published by the Free Software Foundation; either

Btw, as license I would previer LGPLv2+ (as it currently is stated) +e.V. option to relicense.

> inlinenoteinterface.h:71
> + * @endcode
> + *
> + * @see InlineNoteProvider

@author Sven Brauch, Michal Srb

> inlinenoteinterface.h:147-148
> +     *
> +     * @param height the height of the line in pixels
> +     * @param font the font used by the editor
> +     *

There is only one @param note

> inlinenoteinterface.h:199
> +     */
> +    virtual void inlineNoteFocusChanged(const InlineNote& note, bool hasFocus, const QPoint& pos);
> +

I would prefer two functions:

  virtual void inlineNoteFocusInEvent(const InlineNote& note, const QPoint& pos);
  virtual void inlineNoteMoveEvent(const InlineNote& note, const QPoint& pos);
  virtual void inlineNoteFocusOutEvent(const InlineNote& note);

> inlinenoteinterface.h:215
> +
> +class KTEXTEDITOR_EXPORT InlineNote
> +{

API docs missing for InlineNote, but we can add that later.

> inlinenoteinterface.h:320
> +private:
> +    InlineNoteProvider* m_provider;
> +    const KTextEditor::View* m_view;

please initialize all members in-class.

> michalsrb wrote in inlinenoteinterface.h:145
> Perhaps this could be `QVarLengthArray` of some size too?

We discussed that here, and let's use QVector for now. If this turns out to be an issue, we will change for KF6.

> ktexteditor.cpp:238
> +
> +InlineNote::InlineNote()
> +    : m_provider(nullptr)

When members are initialized in the header file, you can write here:

  InlineNote::InlineNote() = default;

REPOSITORY
  R39 KTextEditor

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

To: brauch, michalsrb, dhaumann, cullmann
Cc: anthonyfieroni, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars, dhaumann
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kwrite-devel/attachments/20180816/cfc5cd05/attachment.html>