D18164: Review KateGotoBar

[Dominik Haumann]

dhaumann added a comment.


  I like the idea to go to next modified line up / down. I am undecided about the goto line in clipboard, since I have the feeling it adds more clutter than it helps by default: I think <CTRL+G> <CTRL+V> <ENTER> is quite fast. In addition, this leads to having "Gehe zu" twice in the ui, which is a bit confusing imho. Other opinions?

INLINE COMMENTS

> katedialogs.cpp:1099
> +    btn->setIcon(QIcon::fromTheme(QStringLiteral("go-up-search")));
> +    btn->setText(QStringLiteral(""));
> +    btn->installEventFilter(this);

Please always use QString() in favor of QStringLiteral("")

> katedialogs.cpp:1156
> +        message->setAutoHide(2000); // FIXME Timer is not started as I would expect, only after some "unclear" event
> +        message->setPosition(KTextEditor::Message::BottomInView);
> +        m_view->document()->postMessage(message);

You should also call message->setView(m_view), otherwise the message appears in all views, in case you have multiple views of a document.

> katedialogs.h:103
> +    QSpinBox *m_gotoRange = nullptr;
> +    QToolButton *m_ModifiedUp = nullptr;
> +    QToolButton *m_ModifiedDown = nullptr;

Could you use lowercase naming here? m_modifiedUp instead of m_ModifiedUp?

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

To: loh.tar, #ktexteditor
Cc: dhaumann, cullmann, anthonyfieroni, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, bruns, demsking, sars
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190115/85df7600/attachment.html>