loh.tar updated this revision to Diff 46411. loh.tar added a comment.
I think that's all. Well, it looks less beneficial as previous anticipated by me, but I think it's still the right direction. Just a view questions regarding the mix in the KTextEditor::ViewPrivate code how to access some data: - doc() vs m_doc: doc() is inline - cursorPosition() vs m_viewInternal->m_cursor: cursorPosition() is NOT inline I may have used in my patch also a mix due to the variety of existing code. Let me know which one to use. Furthermore I offer a S&R to unify the existing code, as an own DIFF/ticket or if you like in this one. Normally I would prefer the function calls, but because the highliting looks so nice :-) and cursorPosition() is not inline I'm undecided. Would it be make sense to change them to inline? Some more things I noticed and offer to change. - KateViewInternal::updateView (slot) calls KateViewInternal::doUpdateView (nowhere else called). Any benefit from that construct? Think its only a leftover from the ancient. (would make sense to include in this patch) - KateViewInternal::renderer() (often used) just returns KTextEditor::ViewPrivate::m_renderer but is NOT inline. When removed, could the call "renderer()" replaced by "m_view->m_renderer" or "m_view->renderer()" (the latter is also NOT inline) KateViewInternal contains some additional classes: CalculatingCursor, BoundedCursor, WrappingCursor, together ~275 lines ZoomEventFilter ~53 lines At least the cursor stuff looks worth to move into an own file. Some out-commented FIXME KF5 stuff (4 years old) encapsulated in #ifndef QT_NO_ACCESSIBILITY, lines ~1921 and ~747. May removed in above S&R offer when commit is named "Cleanup" or similar. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17193?vs=46327&id=46411 REVISION DETAIL https://phabricator.kde.org/D17193 AFFECTED FILES src/view/kateview.cpp src/view/kateviewinternal.cpp src/view/kateviewinternal.h To: loh.tar, #ktexteditor Cc: cullmann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, bruns, demsking, sars, dhaumann
