D22261: Add a global shortcut action to turn off the screen

[Kai Uwe Broulik]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20190704/a8afde56/attachment-0001.html>