<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/122249/">https://git.reviewboard.kde.org/r/122249/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 2nd, 2015, 7:47 a.m. UTC, <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/122249/diff/6/?file=351945#file351945line46" style="color: black; font-weight: bold; text-decoration: underline;">processui/keyboardshortcututil.cpp</a>
<span style="font-weight: normal;">
(Diff revision 6)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">46</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">QList</span><span class="o"><</span><span class="n">QKeySequence</span><span class="o">></span> <span class="n">KeyboardShortcutUtil</span><span class="o">::</span><span class="n">findKeysByComponentAndUniqueName</span><span class="p">(</span><span class="n">QString</span> <span class="n">componentPath</span><span class="p">,</span> <span class="n">QString</span> <span class="n">uniqueName</span><span class="p">,</span> <span class="n">QString</span><span class="o">*</span> <span class="n">whyEmptyMessage</span><span class="p">)</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This looks to complicated. It should be much easier to do with the KGlobalAccel API:
<em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;"> create a KActionCollection for component "kwin"
</em> add a QAction with the shortcut name you want
* ask KGlobalAccel to load the shortcut for it.</p></pre>
</blockquote>
<p>On March 8th, 2015, 10:14 a.m. UTC, <b>Gregor Mi</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Thanks for the hint but I am not sure of how to use the API in such a way. I tried two things:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">1)
org::kde::KGlobalAccel kglobalaccel("org.kde.kglobalaccel", "/kglobalaccel", bus);</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%">auto kwinActions = kglobalaccel.allActionsForComponent(QStringList("kwin"));
Q_FOREACH(auto aaa, kwinActions.value()) {
//qDebug() << aaa; // ("kwin", "Kill Window", "KWin", "Kill Window")
}
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Then I wonder how to feed the QStringList to a KActionCollection.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">2)
KActionCollection ac(nullptr, QString());
ac.setComponentName("kwin");
// ac.importGlobalShortcuts();
Q_FOREACH(auto bbb, ac.actions()) {
if (bbb->text() == "Kill Window") {
qDebug() << bbb;
}
}
But the ac.actions() list is empty.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Which of the two ways should be used?</p></pre>
</blockquote>
<p>On March 8th, 2015, 10:25 p.m. UTC, <b>Thomas Lübking</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">tried this?
-----------
KActionCollection ac(this, "kwin");
ac.setConfigGlobal(true);
QAction *act = ac.action("Kill Window");</pre>
</blockquote>
<p>On March 9th, 2015, 5:43 p.m. UTC, <b>Gregor Mi</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I have no QObject* as parent. Can this be the cause?
KActionCollection ac(nullptr, "kwin");
ac.setConfigGlobal(true);
auto killWindowAction = ac.action("Kill Window");
qDebug() << ac.actions().count();
qDebug() << killWindowAction;
/*
RESULT:
0
QObject(0x0)
*/</pre>
</blockquote>
<p>On March 11th, 2015, 10:12 p.m. UTC, <b>Thomas Lübking</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">You'd have "qApp", but I don't actually think so.</p></pre>
</blockquote>
<p>On March 12th, 2015, 7:57 a.m. UTC, <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">try: ac.addAction instead of ac.action</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">For reference code look at e.g. kwin/effects/desktopgrid/desktopgrid_config.cpp</p></pre>
</blockquote>
<p>On March 13th, 2015, 12:45 p.m. UTC, <b>Gregor Mi</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I tried this:
KActionCollection ac(nullptr, "kwin");
ac.setConfigGroup("default"); // needed?
ac.setConfigGlobal(true);
auto killWindowAction = ac.addAction("Kill Window");
killWindowAction->setProperty("isConfigurationAction", true); // neded?
qDebug() << ac.actions().count();
qDebug() << KGlobalAccel::self()->shortcut(killWindowAction).count();
/*
CONSOLE OUTPUT:
1
0
*/</pre>
</blockquote>
<p>On March 13th, 2015, 1:13 p.m. UTC, <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I think you need to register killWindowAction with KGlobalAccel in some way so that it loads the configured shortcut.</p></pre>
</blockquote>
<p>On March 13th, 2015, 1:44 p.m. UTC, <b>Gregor Mi</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I found out how to prepare the QAction without KActionCollection.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">auto killWindowAction = new QAction(nullptr);
killWindowAction->setProperty("componentName", "kwin"); // see impl of KActionCollection
killWindowAction->setObjectName("Kill Window"); // see impl of KActionCollection
// killWindowAction->setProperty("isConfigurationAction", true); // neded?
qDebug() << killWindowAction->objectName(); // --> "Kill Window"
// qDebug() << ac.actions().count(); // --> 1
qDebug() << KGlobalAccel::self()->isComponentActive("kwin"); // --> true
qDebug() << KGlobalAccel::self()->allActionsForComponent({ "kwin" }).count(); // (deprecated) --> 152
qDebug() << KGlobalAccel::self()->hasShortcut(killWindowAction); // --> false
qDebug() << KGlobalAccel::self()->shortcut(killWindowAction).count(); // --> 0
qDebug() << KGlobalAccel::self()->defaultShortcut(killWindowAction).count(); // --> 0
delete killWindowAction;</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Open issues:
1. Is it necessary to specify the shortcut context ("default") somewhere?
2. How to tell KGlobalAccel to load the shortcuts for the given action.</p></pre>
</blockquote>
<p>On March 13th, 2015, 1:49 p.m. UTC, <b>Gregor Mi</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Issue 1.: <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">not</em> necessary, see kde/workspace/plasma-workspace/kglobalaccel/component.h, line 98</p></pre>
</blockquote>
<p>On March 13th, 2015, 1:50 p.m. UTC, <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">concerning 2: see http://api.kde.org/frameworks-api/frameworks5-apidocs/kglobalaccel/html/classKGlobalAccel.html#a66ce504227e7e563f24de4c6b26b0395</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">shortcut context is (I think) not needed.</p></pre>
</blockquote>
<p>On March 13th, 2015, 1:54 p.m. UTC, <b>Gregor Mi</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I read the API documentation about setShortcut and frankly I do not fully understand it. It seems counter-intuitive to me to call that method in order to <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">load</em> the shortcut.</p></pre>
</blockquote>
<p>On March 13th, 2015, 2:03 p.m. UTC, <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">yes the API is weird. But it's really like it: if one doesn't pass NoAutloading the shortcut gets loaded.</p></pre>
</blockquote>
<p>On March 13th, 2015, 2:20 p.m. UTC, <b>Gregor Mi</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">yes, you are right, it is really working like this:
KActionCollection ac(nullptr, "kwin");
auto killWindowAction = ac.addAction("Kill Window");
//killWindowAction->setProperty("componentName", "kwin"); // see impl of KActionCollection
//killWindowAction->setObjectName("Kill Window"); // see impl of KActionCollection
qDebug() << killWindowAction->objectName(); // --> "Kill Window"
qDebug() << KGlobalAccel::self()->hasShortcut(killWindowAction); // --> true
KGlobalAccel::self()->setShortcut(killWindowAction, { }); // KGlobalAccel::Autoloading is default
auto shortcut = KGlobalAccel::self()->shortcut(killWindowAction);
qDebug() << shortcut.count() ; // --> 1
if (shortcut.count() > 0) {
qDebug() << shortcut; // --> Ctrl+Alt+Esc
}
Thanks!
What do you think about adding an API function `loadShortcut(QString componentName, QString actionName)`? (Or just add this example to the documentation of the shortcut() method)</pre>
</blockquote>
<p>On March 13th, 2015, 7:24 p.m. UTC, <b>Gregor Mi</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Don't count your chickens before they are hatched.
With the following reworked method I now observe that the global shortcut stops working (when the actual keyboard shortcut is pressed) although the correct shortcut is returned. I have to relogin to make it work again.
QList<QKeySequence> KeyboardShortcutUtil::findGlobalShortcutByComponentAndActionName(const QString& componentName, const QString& uniqueName)
{
KActionCollection ac(nullptr, componentName); // e.g. "kwin"
auto killWindowAction = ac.addAction(uniqueName); // e.g. "Kill Window"
//interally the following will be done:
// killWindowAction->setProperty("componentName", "kwin"); // see impl of KActionCollection
// killWindowAction->setObjectName("Kill Window"); // see impl of KActionCollection
// we need to call this to load the settings in order to make the shortcut avaible
KGlobalAccel::self()->setShortcut(killWindowAction, { }); // KGlobalAccel::Autoloading is default
// TODO: this will disabled the global shortcut (though it is still shown in KDE Global Keyboard Shortcuts dialog)
// retrieve shortcut list
auto shortcutList = KGlobalAccel::self()->shortcut(killWindowAction);
// TODO: set the shortcut list to reenable it (does not work; you have to relogin)
KGlobalAccel::self()->setShortcut(killWindowAction, shortcutList); // also does not work
return shortcutList;
}</pre>
</blockquote>
<p>On March 13th, 2015, 8:55 p.m. UTC, <b>Thomas Lübking</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">looking at kglobalaccl sources, this:</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">KGlobalAccel::self()->setShortcut(killWindowAction, { });</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">should cause this</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">auto shortcutList = KGlobalAccel::self()->shortcut(killWindowAction);</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">to be "{}" ?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Instead query bool KGlobalAccel::isComponentActive("kwin"), cause otherwise the proper shortcut is "none" anyway ;-)</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">looking at kglobalaccl sources, this:</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">KGlobalAccel::self()->setShortcut(killWindowAction, { });</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">should cause this</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">auto shortcutList = KGlobalAccel::self()->shortcut(killWindowAction);</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">to be "{}" ?</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I thought so too when I looked at the code but it actually returns the correct shortcut (e.g. Ctrl+Alt+Esc). But it also causes that this shortcut stops working when it is pressed on the keyboard.</p></pre>
<br />
<p>- Gregor</p>
<br />
<p>On March 13th, 2015, 7:29 p.m. UTC, Gregor Mi wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Base Apps, Martin Gräßlin and John Tapsell.</div>
<div>By Gregor Mi.</div>
<p style="color: grey;"><i>Updated March 13, 2015, 7:29 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
libksysguard
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Current situation:
The "End Process..." button has a tooltip which says "To target a specific window to kill, press Ctrl+Alt+Esc at any time." The keyboard shortcut is hardcoded.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">New:
Replace the "End Process..." button with a drop-down button and a action named "Kill a specific window..."</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>tests/keyboardshortcututiltest.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>tests/CMakeLists.txt <span style="color: grey">(967b03fae1e460bfb22e1a07ef05cf7b49412546)</span></li>
<li>processui/keyboardshortcututil.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>processui/CMakeLists.txt <span style="color: grey">(7f87b85e0201e63d69070a71203bbb34851a79c6)</span></li>
<li>tests/keyboardshortcututiltest.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>processui/ksysguardprocesslist.cpp <span style="color: grey">(450ca600b8aed7ca611ec638610b6c524c96080c)</span></li>
<li>processui/keyboardshortcututil.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>processui/ProcessWidgetUI.ui <span style="color: grey">(e50f55cf1813b00d49b1716023df487ffbd536e3)</span></li>
<li>CMakeLists.txt <span style="color: grey">(5352e70f6f37daae76c1b3e2c1c9f149d235e3cd)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/122249/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/01/28/16301e88-e21b-4358-9a63-a85dae5722bd__screenshot_default1.png">New End Process button with drop down arrow</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/01/28/58df12c5-7350-4bb0-b602-c5716caa9836__screenshot_default2.png">Drop down shows Kill Window</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>