D25223: Avoid side effects during menu initialization

Kai Uwe Broulik noreply at phabricator.kde.org
Tue Nov 12 08:10:14 GMT 2019


broulik added inline comments.

INLINE COMMENTS

> kdeplatformsystemtrayicon.cpp:27
>  #include <QApplication>
> +#include <QVariant>
>  

Already included in the header file

> kdeplatformsystemtrayicon.cpp:33
>      : QPlatformMenu()
> -    , m_enabled(true)
> -    , m_visible(true)
> -    , m_separatorsCollapsible(true)
> +    , m_enabled(QVariant::Bool)
> +    , m_visible(QVariant::Bool)

Is this explicit initialization neccessary?

> kdeplatformsystemtrayicon.cpp:188
> +    if (!m_enabled.isNull()) {
> +        m_menu->setEnabled(m_enabled.value<bool>());
> +    }

Prefer the specific methods, if exist, `toBool()`

> kdeplatformsystemtrayicon.h:60
>      QIcon m_icon;
> -    bool m_enabled;
> -    bool m_visible;
> -    bool m_separatorsCollapsible;
> +    QVariant m_enabled;
> +    QVariant m_visible;

(This looks like a job for `std::optional`?)

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

To: kmaterka, #plasma, #frameworks, broulik
Cc: cgiboudeaux, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20191112/270b0568/attachment.html>


More information about the Plasma-devel mailing list