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