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