[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    [PATCH] Unbreak global shortcuts configuration
From:       Aurélien Gâteau <aurelien.gateau () free ! fr>
Date:       2008-01-27 20:39:15
Message-ID: fniq5k$tbr$1 () ger ! gmane ! org
[Download RAW message or body]

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

["0_dont_loose_shortcuts_on_close.diff" (text/x-patch)]

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<QObject*>(0));
         }
         QDBusReply<QList<int> > 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())


["1_kdebase_show_empty_actions.diff" (text/x-patch)]

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<QList<int> > l = iface->call("allKeys");
-    QHash<QString,int> shortcuts;
-    foreach( int i, l.value() )
-    {
-        QDBusReply<QStringList> 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<QObject*>(0));
-        }
-        QDBusReply<QList<int> > 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<QStringList> 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<QObject*>(0));
+        actionCollections[component] = col;
+
+        QDBusReply<QStringList> actions = iface->call("allActionsForComponent", \
qVariantFromValue(component) ); +        foreach(const QString &actionText, \
actions.value() )  {
-            if( s.startsWith(title) )
-            {
-                qobject_cast<KAction*>(ac->action(s))->setGlobalShortcut(KShortcut(shortcuts[s]));
                
-                qobject_cast<KAction*>(ac->action(s))->setGlobalShortcutAllowed(true,KAction::NoAutoloading);
 +            kDebug() << "- action:" << actionText;
+            QStringList actionId;
+            actionId << component << actionText;
+            QDBusReply<QList<int> > 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);
             }
         }
     }


["1_kdelibs_show_empty_actions.diff" (text/x-patch)]

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<int, actionData *> keyToAction;
     QHash<QString, QHash<QString, actionData *> *> mainComponentHashes;
-    QList<actionData *> 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<int> KdedGlobalAccel::allKeys()
 {
     QList<int> ret = d->keyToAction.keys();
@@ -266,10 +275,9 @@ QList<int> KdedGlobalAccel::setShortcut(const QStringList &actionId,
     //now we are actually changing the shortcut of the action
 
     QList<int> 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<int> 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<int>();
-    }
-
     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<QString, actionData*> 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<QVariant>&);
     ~KdedGlobalAccel();
 
+    QStringList allComponents();
+    QStringList allActionsForComponent(const QString& component);
+
     QList<int> 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<KdedGlobalAccel *>(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<int> allKeys()
         { return p()->allKeys(); }
-- 
1.5.3.5



["2_kdebase_default_shortcuts.diff" (text/x-patch)]

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<QList<int> > 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<QList<int> > defaultShortcut = iface->call("defaultShortcut", \
qVariantFromValue(actionId)); +            QDBusReply<QList<int> > 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);  }
         }
     }


["2_kdelibs_default_shortcuts.diff" (text/x-patch)]

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<int> keys;
+    QList<int> defaultKeys;
 };
 
 enum IdField
@@ -246,6 +247,15 @@ QList<int> KdedGlobalAccel::shortcut(const QStringList &action)
 }
 
 
+QList<int> KdedGlobalAccel::defaultShortcut(const QStringList &action)
+{
+    actionData *ad = d->findAction(action);
+    if (ad)
+        return ad->defaultKeys;
+    return QList<int>();
+}
+
+
 //TODO: make sure and document that we don't want trailing zero shortcuts in the list
 QList<int> KdedGlobalAccel::setShortcut(const QStringList &actionId,
                                         const QList<int> &keys, uint flags)
@@ -254,6 +264,7 @@ QList<int> 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<int> 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<QVariant>&);
@@ -52,6 +53,8 @@ public:
     QStringList actionId(int key);
     //to be called by main components not owning the action
     QList<int> shortcut(const QStringList &actionId);
+    //to be called by main components not owning the action
+    QList<int> defaultShortcut(const QStringList &actionId);
     //to be called by main components owning the action
     QList<int> setShortcut(const QStringList &actionId,
                            const QList<int> &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<int> shortcut(const QStringList &actionId)
         { return p()->shortcut(actionId); }
+    //get the default keys registered to action
+    inline QList<int> defaultShortcut(const QStringList &actionId)
+        { return p()->defaultShortcut(actionId); }
     //to be called by main components owning the action
     inline QList<int> setShortcut(const QStringList &actionId, const QList<int> &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<int> result = iface.setShortcut(actionId,
                                           intListFromShortcut(action->globalShortcut()),
-- 
1.5.3.5




[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic