Review Request 122249: libksysguard: add Kill Window to End Process button and show correct keyboard shortcut
Thomas Lübking
thomas.luebking at gmail.com
Mon Mar 16 13:40:54 GMT 2015
> On März 2, 2015, 7:47 vorm., Martin Gräßlin wrote:
> > processui/keyboardshortcututil.cpp, line 46
> > <https://git.reviewboard.kde.org/r/122249/diff/6/?file=351945#file351945line46>
> >
> > This looks to complicated. It should be much easier to do with the KGlobalAccel API:
> > * create a KActionCollection for component "kwin"
> > * add a QAction with the shortcut name you want
> > * ask KGlobalAccel to load the shortcut for it.
>
> Gregor Mi wrote:
> Thanks for the hint but I am not sure of how to use the API in such a way. I tried two things:
>
> 1)
> org::kde::KGlobalAccel kglobalaccel("org.kde.kglobalaccel", "/kglobalaccel", bus);
>
> auto kwinActions = kglobalaccel.allActionsForComponent(QStringList("kwin"));
> Q_FOREACH(auto aaa, kwinActions.value()) {
> //qDebug() << aaa; // ("kwin", "Kill Window", "KWin", "Kill Window")
> }
> Then I wonder how to feed the QStringList to a KActionCollection.
>
> 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.
>
> Which of the two ways should be used?
>
> Thomas Lübking wrote:
> tried this?
> -----------
> KActionCollection ac(this, "kwin");
> ac.setConfigGlobal(true);
> QAction *act = ac.action("Kill Window");
>
> Gregor Mi wrote:
> 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)
> */
>
> Thomas Lübking wrote:
> You'd have "qApp", but I don't actually think so.
>
> Martin Gräßlin wrote:
> try: ac.addAction instead of ac.action
>
> For reference code look at e.g. kwin/effects/desktopgrid/desktopgrid_config.cpp
>
> Gregor Mi wrote:
> 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
> */
>
> Martin Gräßlin wrote:
> I think you need to register killWindowAction with KGlobalAccel in some way so that it loads the configured shortcut.
>
> Gregor Mi wrote:
> I found out how to prepare the QAction without KActionCollection.
>
> 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;
>
> 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.
>
> Gregor Mi wrote:
> Issue 1.: *not* necessary, see kde/workspace/plasma-workspace/kglobalaccel/component.h, line 98
>
> Martin Gräßlin wrote:
> concerning 2: see http://api.kde.org/frameworks-api/frameworks5-apidocs/kglobalaccel/html/classKGlobalAccel.html#a66ce504227e7e563f24de4c6b26b0395
>
> shortcut context is (I think) not needed.
>
> Gregor Mi wrote:
> 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 *load* the shortcut.
>
> Martin Gräßlin wrote:
> yes the API is weird. But it's really like it: if one doesn't pass NoAutloading the shortcut gets loaded.
>
> Gregor Mi wrote:
> 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)
>
> Gregor Mi wrote:
> 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;
> }
>
> Thomas Lübking wrote:
> looking at kglobalaccl sources, this:
> > KGlobalAccel::self()->setShortcut(killWindowAction, { });
>
> should cause this
> > auto shortcutList = KGlobalAccel::self()->shortcut(killWindowAction);
>
> to be "{}" ?
>
> Instead query bool KGlobalAccel::isComponentActive("kwin"), cause otherwise the proper shortcut is "none" anyway ;-)
>
> Gregor Mi wrote:
> > looking at kglobalaccl sources, this:
> > > KGlobalAccel::self()->setShortcut(killWindowAction, { });
>
> > should cause this
> > > auto shortcutList = KGlobalAccel::self()->shortcut(killWindowAction);
>
> > to be "{}" ?
>
> 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.
>
> Gregor Mi wrote:
> Trying to make it work with the KDE Global Keyboardshortcuts dialog has no effect. I have to relogin. Maybe some conflict between my KDE 4 and KF5?
>
> Gregor Mi wrote:
> KGlobalAccel::isComponentActive("kwin") is always true when I put it on different places in the above code listing.
>
> Thomas Lübking wrote:
> Yes, because kwin is running ;-)
> The question is, whether this does also make KGlobalAccel::self()->shortcut(killWindowAction) return a usable list (and w/o the "need" to disable the shortcut first)
>
> Gregor Mi wrote:
> Ah. ok. I removed the setShortcut calls but the isComponentActive has no effect. The list is empty.
>
> Gregor Mi wrote:
> I now also tried to modify KGlobalAccel itself and ran into this build error:
>
> CMake Error at CMakeLists.txt:31 (kf5_add_kdeinit_executable)
>
> Is it supposed not to build separate from the plasma-workspace module?
>
> Gregor Mi wrote:
> Sorry for the noise: I took the wrong KGlobalAccel project (there is one in frameworks and one in plasma-workspace)
>
> Gregor Mi wrote:
> [SOLVED] I added this method to KGlobalAccel:
>
> /**
> * See documentation of KGlobalAccel::Autoloading:
> * "Look up the action in global settings (using its main component's name and text())
> * and set the shortcut as saved there."
> */
> void KGlobalAccel::loadShortcutFromGlobalSettings(QAction *action)
> {
> d->updateGlobalShortcut(action, KGlobalAccelPrivate::ActiveShortcut, KGlobalAccel::Autoloading);
> }
>
> When I call this in KeyboardShortcutUtil::findGlobalShortcutByComponentAndActionName instead of setShortcut(...) everything works fine. Though, I am not sure if this fits with the current KGlobalAccel API concept.
>
> Martin Gräßlin wrote:
> I think that method could go in. Maybe more documentation as with the current snipped I don't fully get what it's about ;-)
This call needs to happen unconditioanlly when you require KGlobalAccel::shortcut() - the function advertises to respond the global shortcut, what it apparently does not atm.
- Thomas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122249/#review76861
-----------------------------------------------------------
On März 13, 2015, 10:08 nachm., Gregor Mi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122249/
> -----------------------------------------------------------
>
> (Updated März 13, 2015, 10:08 nachm.)
>
>
> Review request for KDE Base Apps, Martin Gräßlin and John Tapsell.
>
>
> Repository: libksysguard
>
>
> Description
> -------
>
> 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.
>
> New:
> Replace the "End Process..." button with a drop-down button and a action named "Kill a specific window..."
>
>
> Diffs
> -----
>
> CMakeLists.txt 5352e70f6f37daae76c1b3e2c1c9f149d235e3cd
> processui/CMakeLists.txt 7f87b85e0201e63d69070a71203bbb34851a79c6
> processui/ProcessWidgetUI.ui e50f55cf1813b00d49b1716023df487ffbd536e3
> processui/keyboardshortcututil.h PRE-CREATION
> processui/keyboardshortcututil.cpp PRE-CREATION
> processui/ksysguardprocesslist.cpp 450ca600b8aed7ca611ec638610b6c524c96080c
> tests/CMakeLists.txt 967b03fae1e460bfb22e1a07ef05cf7b49412546
> tests/keyboardshortcututiltest.h PRE-CREATION
> tests/keyboardshortcututiltest.cpp PRE-CREATION
>
> Diff: https://git.reviewboard.kde.org/r/122249/diff/
>
>
> Testing
> -------
>
>
> File Attachments
> ----------------
>
> New End Process button with drop down arrow
> https://git.reviewboard.kde.org/media/uploaded/files/2015/01/28/16301e88-e21b-4358-9a63-a85dae5722bd__screenshot_default1.png
> Drop down shows Kill Window
> https://git.reviewboard.kde.org/media/uploaded/files/2015/01/28/58df12c5-7350-4bb0-b602-c5716caa9836__screenshot_default2.png
>
>
> Thanks,
>
> Gregor Mi
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20150316/72255216/attachment.htm>
More information about the kde-core-devel
mailing list