broulik added a comment.
Thanks a lot for your patch! I have some minor style nitpicks (you probably just copied them from elsewhere but if we're adding new code, it should be tidy ;) Bonus points if you also make a patch for kscreenlocker globalaccel.cpp to whitelist this new action so that it also works on the lock screen. INLINE COMMENTS > powerdevildpmsaction.cpp:42 > > +#include <kglobalaccel.h> > + Use `#include <KGlobalAccel>` and sort it correctly into the other includes > powerdevildpmsaction.cpp:78 > + > + KActionCollection* actionCollection = new KActionCollection( this ); > + actionCollection->setComponentDisplayName(i18nc("Name for powerdevil > shortcuts category", "Power Management")); Coding style: `KActionCollection *actionCollection` (asterisk after space) > powerdevildpmsaction.cpp:83 > + globalAction->setText(i18nc("@action:inmenu Global shortcut", "Turn Off > Screen")); > + accel->setGlobalShortcut(globalAction, QList<QKeySequence>()); > + connect(globalAction, SIGNAL(triggered(bool)), SLOT(turnOffScreen())); I suppose there's no `Qt::Key_` enum for that, given laptops typically bypass the operating system here anyway. > powerdevildpmsaction.cpp:84 > + accel->setGlobalShortcut(globalAction, QList<QKeySequence>()); > + connect(globalAction, SIGNAL(triggered(bool)), SLOT(turnOffScreen())); > } Use new style connect, you can probably just use a lambda connect(globalAction, &QAction::triggered, this, [this] { if (m_helper) { m_helper->trigger(QStringLiteral("TurnOff")); } }); > powerdevildpmsaction.h:57 > void setKeyboardBrightnessHelper(int brightness); > + void turnOffScreen(); > For `SLOT(...)` syntax to work this must be in a section `private Q_SLOTS`, but note my comment below about using a lambda instead REPOSITORY R122 Powerdevil REVISION DETAIL https://phabricator.kde.org/D22261 To: mblumenstingl, #plasma, broulik Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart