D10297: Add new "Tools" button above System Monitor's process list

[gregormi]

gregormi added a comment.


  In D10297#272726 <https://phabricator.kde.org/D10297#272726>, @rkflx wrote:
  
  > Awesome, I did not expect that the fix for the shortcuts would be so simple. Thanks for your patience, we are nearly there…

INLINE COMMENTS

> rkflx wrote in ksysguardprocesslist.cpp:421-427
> Left-over debug code?

It was supposed to serve as reminder of how the parameters for the globalShortcut method were determined. To help debugging later. Should it be removed?

> rkflx wrote in ksysguardprocesslist.cpp:429
> Do you think it is really necessary to display `not set`? For me, an empty string would also work just fine, now that you are using `\t` instead of `(…)`.
> 
> Also I found a way to omit the string and `\t` entirely and let Qt do all the work:
> 
>   auto runCommandAction = new QAction(i18nc("@action:inmenu", "Run Command"), this);
>   const auto runCommandShortcutList = KGlobalAccel::self()->globalShortcut(QStringLiteral("krunner"), QStringLiteral("run command"));
>   if (runCommandShortcutList.size() > 0) {
>       runCommandAction->setShortcut(runCommandShortcutList[0]);
>   }

Yes, is this much better.

Might there be a chance of a some kind of keyboard shortcut conflict because we now set it also locally?

REPOSITORY
  R111 KSysguard Library

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

To: gregormi, #plasma, colomar, broulik, mart, hein, rkflx
Cc: apol, anthonyfieroni, andreaska, rkflx, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180629/e3a467fd/attachment.html>