From kde-core-devel Sun Jan 27 20:39:15 2008 From: =?UTF-8?B?QXVyw6lsaWVuIEfDonRlYXU=?= Date: Sun, 27 Jan 2008 20:39:15 +0000 To: kde-core-devel Subject: [PATCH] Unbreak global shortcuts configuration Message-Id: X-MARC-Message: https://marc.info/?l=kde-core-devel&m=120146642203942 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--nextPart1306210.SPYKUYFuaP" --nextPart1306210.SPYKUYFuaP Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8Bit Hello, I announced last week I was working on the global shortcuts kcm and intended to post a patch this monday. Obviously I am a bit late :-), but here are the patches. The patches are against kdelibs and kdebase. I use git-svn for kdelibs and svn+quilt for kdebase right now, so the patches are formatted a bit differently, sorry about that. # 0_dont_loose_shortcut_on_close.diff This is the patch I posted to fix shortcuts disappearing when you close the kcm. It's not elegant, since it involves intentionally leaking the KAction instances so that their destructor do not delete the shortcuts. I include it nevertheless because the other patches are based on it. As I said in my previous message, it could be done in a more elegant way by adding a flag to KAction to disable shortcut deletion when it is deleted. Will probably do it. # 1_{kdelibs,kdebase}_show_empty_actions.diff This patchset brings the empty actions (like KWin actions) back. To do so, I had to rework the way the kcm initialize itself: it used to ask KdedGlobalAccel for all available shortcuts, then for each shortcut get the action. Obviously this implementation couldn't get actions without shortcuts. My implementation ask KdedGlobalAccel for the list of components, then for the list of actions for each component. To do so it adds two methods to KdedGlobalAccel: allComponents() and allActionsForComponent(). I also had to remove all the "remove empty shortcuts" code from KdedGlobalAccel. # 2_{kdelibs,kdebase}_default_shortcuts.diff Previous patchset makes it possible to define shortcuts for all actions, but when you define a custom shortcut, close the kcm and open it again, your custom shortcut will appear as the default one. This second patchset makes KdedGlobalAccel aware of the concept of default and active shortcuts. It adds a new method: defaultShortcut(), which is used by the kcm to initialize the KAction correctly. I guess people will object against applying the patches until #0 is fixed, but I would like to have feedback about the others nevertheless. Aurélien --nextPart1306210.SPYKUYFuaP Content-Type: text/x-patch; name="0_dont_loose_shortcuts_on_close.diff" Content-Transfer-Encoding: 8Bit Content-Description: Content-Disposition: attachment; filename="0_dont_loose_shortcuts_on_close.diff" Index: workspace/kcontrol/keys/globalshortcuts.cpp =================================================================== --- workspace.orig/kcontrol/keys/globalshortcuts.cpp 2008-01-26 20:28:28.000000000 +0100 +++ workspace/kcontrol/keys/globalshortcuts.cpp 2008-01-26 20:28:30.000000000 +0100 @@ -81,14 +81,16 @@ QStringList actionlist = actionid.value(); if( !actionCollections.contains( actionlist.first() ) ) { - actionCollections[actionlist.first()] = new KActionCollection(this); + // No parent for the collection: we don't want the destructor to be + // called, otherwise our action shortcuts will get lost + actionCollections[actionlist.first()] = new KActionCollection(static_cast(0)); } QDBusReply > shortcut = iface->call("shortcut", qVariantFromValue(actionid.value())); - KAction *action = new KAction(actionlist.at(1), this); KActionCollection* col = actionCollections[actionlist.first()]; QString actionname = QString("%1_%2").arg(actionlist.first()).arg(col->count()); + KAction *action = col->addAction(actionname); + action->setText(actionlist.at(1)); shortcuts[actionname] = shortcut.value().first(); - col->addAction(actionname, action); } KComponentData curcomp = KGlobal::mainComponent(); foreach(QString title, actionCollections.keys()) --nextPart1306210.SPYKUYFuaP Content-Type: text/x-patch; name="1_kdebase_show_empty_actions.diff" Content-Transfer-Encoding: 8Bit Content-Disposition: attachment; filename="1_kdebase_show_empty_actions.diff" Index: workspace/kcontrol/keys/globalshortcuts.cpp =================================================================== --- workspace.orig/kcontrol/keys/globalshortcuts.cpp 2008-01-26 20:28:53.000000000 +0100 +++ workspace/kcontrol/keys/globalshortcuts.cpp 2008-01-26 20:29:03.000000000 +0100 @@ -73,36 +73,31 @@ QDBusConnection bus = QDBusConnection::sessionBus(); QDBusInterface* iface = new QDBusInterface( "org.kde.kded", "/KdedGlobalAccel", "org.kde.KdedGlobalAccel", bus, this ); - QDBusReply > l = iface->call("allKeys"); - QHash shortcuts; - foreach( int i, l.value() ) - { - QDBusReply actionid = iface->call("action", qVariantFromValue(i)); - QStringList actionlist = actionid.value(); - if( !actionCollections.contains( actionlist.first() ) ) - { - // No parent for the collection: we don't want the destructor to be - // called, otherwise our action shortcuts will get lost - actionCollections[actionlist.first()] = new KActionCollection(static_cast(0)); - } - QDBusReply > shortcut = iface->call("shortcut", qVariantFromValue(actionid.value())); - KActionCollection* col = actionCollections[actionlist.first()]; - QString actionname = QString("%1_%2").arg(actionlist.first()).arg(col->count()); - KAction *action = col->addAction(actionname); - action->setText(actionlist.at(1)); - shortcuts[actionname] = shortcut.value().first(); - } + QDBusReply components = iface->call("allComponents"); KComponentData curcomp = KGlobal::mainComponent(); - foreach(QString title, actionCollections.keys()) + foreach(const QString &component, components.value() ) { - KGlobalAccel::self()->overrideMainComponentData(KComponentData(title.toAscii())); - KActionCollection* ac = actionCollections[title]; - foreach( QString s, shortcuts.keys() ) + kDebug() << "component:" << component; + KGlobalAccel::self()->overrideMainComponentData(KComponentData(component.toAscii())); + // No parent for the collection: we don't want the destructor to be + // called, otherwise our action shortcuts will get lost + KActionCollection* col = new KActionCollection(static_cast(0)); + actionCollections[component] = col; + + QDBusReply actions = iface->call("allActionsForComponent", qVariantFromValue(component) ); + foreach(const QString &actionText, actions.value() ) { - if( s.startsWith(title) ) - { - qobject_cast(ac->action(s))->setGlobalShortcut(KShortcut(shortcuts[s])); - qobject_cast(ac->action(s))->setGlobalShortcutAllowed(true,KAction::NoAutoloading); + kDebug() << "- action:" << actionText; + QStringList actionId; + actionId << component << actionText; + QDBusReply > shortcut = iface->call("shortcut", qVariantFromValue(actionId)); + QString actionName = QString("%1_%2").arg(component).arg(col->count()); + KAction *action = col->addAction(actionName); + action->setText(actionText); + if (!shortcut.value().empty()) { + int key = shortcut.value().first(); + action->setGlobalShortcut(KShortcut(key)); + action->setGlobalShortcutAllowed(true,KAction::NoAutoloading); } } } --nextPart1306210.SPYKUYFuaP Content-Type: text/x-patch; name="1_kdelibs_show_empty_actions.diff" Content-Transfer-Encoding: 8Bit Content-Disposition: attachment; filename="1_kdelibs_show_empty_actions.diff" diff --git a/kdeui/shortcuts/kdedglobalaccel.cpp b/kdeui/shortcuts/kdedglobalaccel.cpp index dd6a837..e00d587 100644 --- a/kdeui/shortcuts/kdedglobalaccel.cpp +++ b/kdeui/shortcuts/kdedglobalaccel.cpp @@ -88,7 +88,6 @@ public: QHash keyToAction; QHash *> mainComponentHashes; - QList deletionQueue; KConfig config; KConfigGroup configGroup; @@ -204,6 +203,16 @@ KdedGlobalAccel::~KdedGlobalAccel() delete d; } +QStringList KdedGlobalAccel::allComponents() +{ + return d->mainComponentHashes.keys(); +} + +QStringList KdedGlobalAccel::allActionsForComponent(const QString& component) +{ + return d->mainComponentHashes[component]->keys(); +} + QList KdedGlobalAccel::allKeys() { QList ret = d->keyToAction.keys(); @@ -266,10 +275,9 @@ QList KdedGlobalAccel::setShortcut(const QStringList &actionId, //now we are actually changing the shortcut of the action QList added = d->nonemptyOnly(keys); - bool emptyShortcut = added.isEmpty(); bool didCreate = false; - if (!ad && (!emptyShortcut || !isDefaultEmpty)) { + if (!ad) { didCreate = true; ad = d->addAction(actionId); ad->isPresent = false; @@ -334,16 +342,6 @@ QList KdedGlobalAccel::setShortcut(const QStringList &actionId, scheduleWriteSettings(); - if (isDefaultEmpty && d->isEmpty(ad->keys)) { - d->takeAction(actionId); - if (didCreate) - delete ad; - else - d->deletionQueue.append(ad); - - return QList(); - } - return ad->keys; } @@ -390,14 +388,6 @@ void KdedGlobalAccel::scheduleWriteSettings() //slot void KdedGlobalAccel::writeSettings() { - //entries scheduled for deletion are in deletionQueue - foreach (const actionData *const ad, d->deletionQueue) { - QString confKey = ad->actionId.join("\01"); - d->configGroup.deleteEntry(confKey); - delete ad; - } - d->deletionQueue.clear(); - typedef QHash adHash; //avoid comma in macro arguments foreach (const adHash *const mc, d->mainComponentHashes) { foreach (const actionData *const ad, *mc) { diff --git a/kdeui/shortcuts/kdedglobalaccel.h b/kdeui/shortcuts/kdedglobalaccel.h index 76d1bde..c08c2df 100644 --- a/kdeui/shortcuts/kdedglobalaccel.h +++ b/kdeui/shortcuts/kdedglobalaccel.h @@ -43,6 +43,9 @@ public: KdedGlobalAccel(QObject*, const QList&); ~KdedGlobalAccel(); + QStringList allComponents(); + QStringList allActionsForComponent(const QString& component); + QList allKeys(); QStringList allKeysAsString(); diff --git a/kdeui/shortcuts/kdedglobalaccel_adaptor.h b/kdeui/shortcuts/kdedglobalaccel_adaptor.h index 9aa72a9..3bfe157 100644 --- a/kdeui/shortcuts/kdedglobalaccel_adaptor.h +++ b/kdeui/shortcuts/kdedglobalaccel_adaptor.h @@ -52,6 +52,10 @@ private: inline KdedGlobalAccel *p() { return static_cast(parent()); } public Q_SLOTS: + inline QStringList allComponents() + { return p()->allComponents(); } + inline QStringList allActionsForComponent(const QString &component) + { return p()->allActionsForComponent(component); } //get all registered keys (mainly for debugging) inline QList allKeys() { return p()->allKeys(); } -- 1.5.3.5 --nextPart1306210.SPYKUYFuaP Content-Type: text/x-patch; name="2_kdebase_default_shortcuts.diff" Content-Transfer-Encoding: 8Bit Content-Disposition: attachment; filename="2_kdebase_default_shortcuts.diff" Index: workspace/kcontrol/keys/globalshortcuts.cpp =================================================================== --- workspace.orig/kcontrol/keys/globalshortcuts.cpp 2008-01-26 20:30:33.000000000 +0100 +++ workspace/kcontrol/keys/globalshortcuts.cpp 2008-01-26 20:55:39.000000000 +0100 @@ -88,16 +88,25 @@ foreach(const QString &actionText, actions.value() ) { kDebug() << "- action:" << actionText; - QStringList actionId; - actionId << component << actionText; - QDBusReply > shortcut = iface->call("shortcut", qVariantFromValue(actionId)); QString actionName = QString("%1_%2").arg(component).arg(col->count()); KAction *action = col->addAction(actionName); action->setText(actionText); - if (!shortcut.value().empty()) { + + QStringList actionId; + actionId << component << actionText; + QDBusReply > defaultShortcut = iface->call("defaultShortcut", qVariantFromValue(actionId)); + QDBusReply > shortcut = iface->call("shortcut", qVariantFromValue(actionId)); + if (!defaultShortcut.value().empty()) + { + int key = defaultShortcut.value().first(); + kDebug() << "-- defaultShortcut" << KShortcut(key).toString(); + action->setGlobalShortcut(KShortcut(key), KAction::DefaultShortcut, KAction::NoAutoloading); + } + if (!shortcut.value().empty()) + { int key = shortcut.value().first(); - action->setGlobalShortcut(KShortcut(key)); - action->setGlobalShortcutAllowed(true,KAction::NoAutoloading); + kDebug() << "-- shortcut" << KShortcut(key).toString(); + action->setGlobalShortcut(KShortcut(key), KAction::ActiveShortcut, KAction::NoAutoloading); } } } --nextPart1306210.SPYKUYFuaP Content-Type: text/x-patch; name="2_kdelibs_default_shortcuts.diff" Content-Transfer-Encoding: 8Bit Content-Disposition: attachment; filename="2_kdelibs_default_shortcuts.diff" diff --git a/kdeui/shortcuts/kdedglobalaccel.cpp b/kdeui/shortcuts/kdedglobalaccel.cpp index e00d587..3d6bf06 100644 --- a/kdeui/shortcuts/kdedglobalaccel.cpp +++ b/kdeui/shortcuts/kdedglobalaccel.cpp @@ -60,6 +60,7 @@ struct actionData bool isDefaultEmpty : 1; QStringList actionId; QList keys; + QList defaultKeys; }; enum IdField @@ -246,6 +247,15 @@ QList KdedGlobalAccel::shortcut(const QStringList &action) } +QList KdedGlobalAccel::defaultShortcut(const QStringList &action) +{ + actionData *ad = d->findAction(action); + if (ad) + return ad->defaultKeys; + return QList(); +} + + //TODO: make sure and document that we don't want trailing zero shortcuts in the list QList KdedGlobalAccel::setShortcut(const QStringList &actionId, const QList &keys, uint flags) @@ -254,6 +264,7 @@ QList KdedGlobalAccel::setShortcut(const QStringList &actionId, const bool isDefaultEmpty = (flags & IsDefaultEmpty); const bool setPresent = (flags & SetPresent); const bool isAutoloading = !(flags & NoAutoloading); + const bool isDefault = (flags & IsDefault); actionData *ad = d->findAction(actionId); @@ -311,6 +322,8 @@ QList KdedGlobalAccel::setShortcut(const QStringList &actionId, ad->isDefaultEmpty = isDefaultEmpty; if (setPresent) ad->isPresent = true; + if (isDefault) + ad->defaultKeys = keys; ad->keys = keys; //update keyToAction and find conflicts with other actions @@ -392,7 +405,13 @@ void KdedGlobalAccel::writeSettings() foreach (const adHash *const mc, d->mainComponentHashes) { foreach (const actionData *const ad, *mc) { QString confKey = ad->actionId.join("\01"); - if (!d->isEmpty(ad->keys)) + if (ad->keys == ad->defaultKeys) + { + // If this is a default key, make sure we don't keep an old + // custom key in the config file + d->configGroup.deleteEntry(confKey); + } + else if (!d->isEmpty(ad->keys)) d->configGroup.writeEntry(confKey, stringFromKeys(ad->keys)); else d->configGroup.writeEntry(confKey, "none"); diff --git a/kdeui/shortcuts/kdedglobalaccel.h b/kdeui/shortcuts/kdedglobalaccel.h index c08c2df..6b3dfd3 100644 --- a/kdeui/shortcuts/kdedglobalaccel.h +++ b/kdeui/shortcuts/kdedglobalaccel.h @@ -37,7 +37,8 @@ public: { IsDefaultEmpty = 1, SetPresent = 2, - NoAutoloading = 4 + NoAutoloading = 4, + IsDefault = 8 }; KdedGlobalAccel(QObject*, const QList&); @@ -52,6 +53,8 @@ public: QStringList actionId(int key); //to be called by main components not owning the action QList shortcut(const QStringList &actionId); + //to be called by main components not owning the action + QList defaultShortcut(const QStringList &actionId); //to be called by main components owning the action QList setShortcut(const QStringList &actionId, const QList &keys, uint flags); diff --git a/kdeui/shortcuts/kdedglobalaccel_adaptor.h b/kdeui/shortcuts/kdedglobalaccel_adaptor.h index 3bfe157..2985d6d 100644 --- a/kdeui/shortcuts/kdedglobalaccel_adaptor.h +++ b/kdeui/shortcuts/kdedglobalaccel_adaptor.h @@ -68,6 +68,9 @@ public Q_SLOTS: //get the keys registered to action inline QList shortcut(const QStringList &actionId) { return p()->shortcut(actionId); } + //get the default keys registered to action + inline QList defaultShortcut(const QStringList &actionId) + { return p()->defaultShortcut(actionId); } //to be called by main components owning the action inline QList setShortcut(const QStringList &actionId, const QList &keys, uint flags) { return p()->setShortcut(actionId, keys, flags); } diff --git a/kdeui/shortcuts/kglobalaccel.cpp b/kdeui/shortcuts/kglobalaccel.cpp index c41627f..fc47967 100644 --- a/kdeui/shortcuts/kglobalaccel.cpp +++ b/kdeui/shortcuts/kglobalaccel.cpp @@ -152,10 +152,14 @@ void KGlobalAccelPrivate::updateGlobalShortcutAllowed(KAction *action, uint flag if (!oldEnabled && newEnabled) { uint setterFlags = KdedGlobalAccel::SetPresent; + KShortcut defaultShortcut = action->globalShortcut(KAction::DefaultShortcut); + KShortcut activeShortcut = action->globalShortcut(); if (flags & KAction::NoAutoloading) setterFlags |= KdedGlobalAccel::NoAutoloading; - if (action->globalShortcut(KAction::DefaultShortcut).isEmpty()) + if (defaultShortcut.isEmpty()) setterFlags |= KdedGlobalAccel::IsDefaultEmpty; + if (defaultShortcut == activeShortcut) + setterFlags |= KdedGlobalAccel::IsDefault; nameToAction.insert(actionId.at(1), action); actionToName.insert(action, actionId.at(1)); @@ -187,10 +191,14 @@ void KGlobalAccelPrivate::updateGlobalShortcut(KAction *action, uint flags) //TODO: what about i18ned names? uint setterFlags = 0; + KShortcut defaultShortcut = action->globalShortcut(KAction::DefaultShortcut); + KShortcut activeShortcut = action->globalShortcut(); if (flags & KAction::NoAutoloading) setterFlags |= KdedGlobalAccel::NoAutoloading; - if (action->globalShortcut(KAction::DefaultShortcut).isEmpty()) + if (defaultShortcut.isEmpty()) setterFlags |= KdedGlobalAccel::IsDefaultEmpty; + if (defaultShortcut == activeShortcut) + setterFlags |= KdedGlobalAccel::IsDefault; QList result = iface.setShortcut(actionId, intListFromShortcut(action->globalShortcut()), -- 1.5.3.5 --nextPart1306210.SPYKUYFuaP--