D12662: Add InlineNoteInterface

Sven Brauch noreply at phabricator.kde.org
Wed May 2 18:36:50 UTC 2018


brauch added a comment.


  Looks good from the implementation too so far. One thing I do not see is any changes to the cursorToX / xToCursor functions, is there really no change required there?
  
  Some things which come to my mind for testing would be:
  
  - is selection rendered correctly if it includes notes, at the end, beginning, or middle of lines, also mult-line selections?
  - what happens when clicking or dragging from or into the notes?
  - does it still work properly with dynamic word-wrap on?
  - does it work properly with code-folding? what happens if a note is at the border of a folding region?

INLINE COMMENTS

> katedocument.cpp:5295
> +
> +        connect(provider, SIGNAL(reset()), this, SLOT(inlineNotesReset()));
> +        connect(provider, SIGNAL(lineChanged(int)), this, SLOT(inlineNotesLineChanged(int)));

Can you use new-style connect here, i.e. the function-pointer syntax
connect(provider, &NoteProvider::reset, this, &DocumentPrivate::inlineNotesReset)?
This gives compile-time argument type checking.

> katerenderer.cpp:771
> +            // Determine the position where to paint the note.
> +	    // We start by getting the x coordinate of cursor placed to the column.
> +            qreal x = range->viewLine(viewLine).lineLayout().cursorToX(column) - xStart;

indent

> katerenderer.cpp:777
> +                // note should be painted and the cursor gets placed at the right side of it. So we have to
> +		// subtract the width of the note to get to left side of the hole.
> +                x -= inlineNote->width(lineHeight(), currentFontMetrics());

indent

REPOSITORY
  R39 KTextEditor

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

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


More information about the Kde-frameworks-devel mailing list