dfaure accepted this revision.
dfaure added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> dropjob.cpp:330
> +        }
> +        //separator at last position and not needed anymore?
> +        if (d->m_appActions.isEmpty() && d->m_pluginActions.isEmpty() &&

Don't worry too much about that, QMenu automatically suppresses doubled 
separators or separators at begin/end of the menu.
So I bet this if block isn't necessary in practice.

> dropjob.cpp:332
> +        if (d->m_appActions.isEmpty() && d->m_pluginActions.isEmpty() &&
> +            menu->actions().last()->isSeparator()) {
> +            menu->removeAction(menu->actions().last());

If you do keep it, maybe add a check that menu->actions() isn't empty, 
otherwise last() will assert.

REPOSITORY
  R241 KIO

BRANCH
  phab/setActionsLater

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: mart, #plasma, dfaure
Cc: dfaure, hein, plasma-devel, #frameworks, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

Reply via email to