D22424: TextDocument: remove actions from contextmenu on hide already

[René J.V. Bertin]

rjvbb added inline comments.

INLINE COMMENTS

> textdocument.cpp:760
> +    auto* menu = qobject_cast<QMenu*>(sender());
> +    Q_ASSERT(menu == d->currentContextMenu);
> +

Why take the risk of hard to understand behaviour in production builds? IMO just make this a runtime `if` if you cannot prove right here and now that the ASSERT is redundant. The gain of skipping the if is purely academic (and failing the test doesn't justify causing an abort in debug builds either; a qWarning() would do just fine).

> textdocument.cpp:771
>  
> -    if (d->addedContextMenu) {
> -        const auto actions = d->addedContextMenu->actions();
> -        for (QAction* action : actions) {
> -            menu->removeAction(action);
> -        }
> -        delete d->addedContextMenu;
> -    }
> +    Q_ASSERT(!d->addedContextMenu);
> +

Idem, just call unpopulateContextMenu().

REPOSITORY
  R32 KDevelop

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

To: kossebau, #kdevelop
Cc: anthonyfieroni, rjvbb, kdevelop-devel, hmitonneau, christiant, glebaccon, domson, antismap, iodelay, alexeymin, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20190713/7183d637/attachment.html>