D7458: Show icons in icon border context menu

[Dominik Haumann]

dhaumann requested changes to this revision.
dhaumann added a comment.
This revision now requires changes to proceed.


  I think we need one more iteration, then this is good to go, see the comments.

INLINE COMMENTS

> kfunk wrote in kateviewhelpers.cpp:2253
> Less code with:
> 
>   if (auto icon = ...) {
>       mA->setIcon(icon);
>       dMA->setIcon(icon);
>   }

What most likely would work is this:

  if (!m_doc->markDescription(markType).isEmpty()) {
      const QPixmap icon = m_doc->markPixmap(markType);
      mA = markMenu.addAction(icon, m_doc->markDescription(markType));
      dMA = selectDefaultMark.addAction(icon, m_doc->markDescription(markType));
  } else {
      mA = markMenu.addAction(i18n("Mark Type %1",  bit + 1));
      dMA = selectDefaultMark.addAction(i18n("Mark Type %1",  bit + 1));
  }

Looking at the Qt code of the constructor QIcon(const QPixmap & ) it becomes clear that a null pixmap will simply create a null icon. And a null QIcon will not be displayed in the gui, so we do not need the if at all, right?

Could you test this?

REPOSITORY
  R39 KTextEditor

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

To: croick, #ktexteditor, dhaumann
Cc: dhaumann, kfunk, kwrite-devel, #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170822/5712637d/attachment.html>