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

List:       kde-core-devel
Subject:    Fixing i18n + global shortcuts
From:       Andreas Hartmetz <ahartmetz () gmail ! com>
Date:       2008-03-03 17:59:33
Message-ID: 200803031859.34250.ahartmetz () gmail ! com
[Download RAW message or body]

Hi all!

I've been hacking on this on and off for some time and it's quite close to 
ready so I want to make it known to avoid having to repeat answers to 
questions on IRC.
This is also a request for comments but I don't expect many if at all :)
The idea is that actions with global shortcuts will internally have "friendly" 
names which might [should] be translated and unique names like those used in 
KActionCollection::addAction(). The objectName() will be used for that 
purpose, similar to KActionCollection().
Other notable changes are:
-KAction::globalShortcutAllowed() will be deprecated and replaced with 
globalShortcutEnabled() according to the patch.
-somewhat better documentation
-default shortcuts will be saved too to facilitate the work of the global 
shortcuts KCM (there is and should be no other user I know of).
-for the time being, there will be no way to "forget" an action. I would like 
to add explicit API to do this instead of doing it using magic parameter 
values [i.e. KShortcut(), NoAutoloading] to KAction::setGlobalShortcut().

There is still room for improvement but overall the new code should be better 
than the old already. Please see the attachment for the largish patch.

Cheers,
Andreas

["fix-globalshortcuts-i18n.patch" (text/x-diff)]

diff -r fcc9e08c2d7a kdeui/actions/kaction.cpp
--- a/kdeui/actions/kaction.cpp	Thu Feb 28 03:33:48 2008 +0000
+++ b/kdeui/actions/kaction.cpp	Thu Feb 28 09:45:49 2008 +0100
@@ -81,14 +81,17 @@ KAction::~KAction()
 KAction::~KAction()
 {
     // When deleting actions in the configuration module, we don't want to
-    // unregister the global shortcut
+    // deactivate the global shortcut
     if (property("isConfigurationAction").toBool() == false) {
-        if(d->globalShortcutAllowed) {
-            //the combination of the following lines essentially
-            //- removes the action from KGlobalAccel
-            //- marks the action as inactive in the KDED module
-            d->globalShortcutAllowed = false;
-            KGlobalAccel::self()->d->updateGlobalShortcutAllowed(this, \
(ShortcutTypes)0); +        if (d->globalShortcutEnabled) {
+            // - remove the action from KGlobalAccel
+            // - mark the action as inactive in the KDED module
+            d->globalShortcutEnabled = false;
+            KGlobalAccel::self()->d->setInactive(this);
+        } else {
+            // we leak memory in KGlobalAccel for this action's data set.
+            // HOWEVER this is a limited leak because if/when this action is
+            // created again there will be no additional memory consumption.
         }
     }
 
@@ -175,6 +178,10 @@ void KAction::setGlobalShortcut( const K
                                  GlobalShortcutLoading load )
 {
   Q_ASSERT(type);
+  if (!d->globalShortcutEnabled) {
+    //return;
+    enableGlobalShortcut();   //backwards compatibility
+  }
 
   bool changed = false;
   if ((type & DefaultShortcut) && d->defaultGlobalShortcut != shortcut) {
@@ -187,32 +194,57 @@ void KAction::setGlobalShortcut( const K
     changed = true;
   }
 
-  if (changed)
+  if (changed) {
     KGlobalAccel::self()->d->updateGlobalShortcut(this, type | load);
-
-  //This *must* be called after setting the shortcut. The shortcut will be fixed as
-  //soon as it has been enabled once. If we called updateGlobalShortcutAllowed too
-  //early, the shortcut would be fixed to empty.
-  if (!d->globalShortcutAllowed) {
-    d->globalShortcutAllowed = true;
-    KGlobalAccel::self()->d->updateGlobalShortcutAllowed(this, type | load);
   }
-
 }
 
-//private
-bool KAction::globalShortcutAllowed( ) const
+bool KAction::globalShortcutAllowed() const
 {
-  return d->globalShortcutAllowed;
+  return d->globalShortcutEnabled;
+}
+
+bool KAction::isGlobalShortcutEnabled() const
+{
+  return d->globalShortcutEnabled;
 }
 
 void KAction::setGlobalShortcutAllowed( bool allowed, GlobalShortcutLoading load )
 {
-  if (d->globalShortcutAllowed != allowed) {
-    d->globalShortcutAllowed = allowed;
+  if (d->globalShortcutEnabled == allowed)
+    return;
+  
+  d->globalShortcutEnabled = allowed;
+  if (allowed) {
+    KGlobalAccel::self()->d->doRegister(this);
+  } else {
+    KGlobalAccel::self()->d->setInactive(this);
+  }
+}
 
-    KGlobalAccel::self()->d->updateGlobalShortcutAllowed(this, load);
-  }
+bool KAction::enableGlobalShortcut()
+{
+    //If the object name was nonempty before and globalShortcutEnabled() == true -
+    //well, you shouldn't have changed the objectName() anyway so we don't check \
that. +    if (objectName().isEmpty()) {
+        return false;
+    }
+    if (d->globalShortcutEnabled) {
+        return true;
+    }
+    d->globalShortcutEnabled = true;
+    KGlobalAccel::self()->d->doRegister(this);
+    return true;
+}
+
+void KAction::disableGlobalShortcut()
+{
+    d->globalShortcut = KShortcut();
+    d->defaultGlobalShortcut = KShortcut();
+    if (d->globalShortcutEnabled) {
+        d->globalShortcutEnabled = false;
+        KGlobalAccel::self()->d->setInactive(this);
+    }
 }
 
 KShapeGesture KAction::shapeGesture( ShortcutTypes type ) const
diff -r fcc9e08c2d7a kdeui/actions/kaction.h
--- a/kdeui/actions/kaction.h	Thu Feb 28 03:33:48 2008 +0000
+++ b/kdeui/actions/kaction.h	Thu Feb 28 09:45:49 2008 +0100
@@ -196,6 +196,7 @@ class KDEUI_EXPORT KAction : public QWid
   Q_PROPERTY( bool shortcutConfigurable READ isShortcutConfigurable WRITE \
setShortcutConfigurable )  Q_PROPERTY( KShortcut globalShortcut READ globalShortcut \
WRITE setGlobalShortcut )  Q_PROPERTY( bool globalShortcutAllowed READ \
globalShortcutAllowed WRITE setGlobalShortcutAllowed ) +  Q_PROPERTY( bool \
globalShortcutEnabled READ isGlobalShortcutEnabled )  Q_FLAGS( ShortcutType )
 
 public:
@@ -337,25 +338,29 @@ public:
 
     /**
      * Assign a global shortcut for this action. Global shortcuts
-     * allow your actions to respond to keys independently of the focused window.
-     * Unlike regular shortcuts, the application's window does not need focus
-     * for them to be activated.
+     * allow an action to respond to key shortcuts independently of the focused \
window, +     * i.e. the action will trigger if the keys were pressed no matter where \
                in the X session.
      *
-     * When an action, identified by main component name and text(), is assigned
+     * This method will do nothing if isGlobalShortcutEnabled() returns false.
+     *
+     * When an action, identified by main component name and objectName(), is \
                assigned
      * a global shortcut for the first time on a KDE installation the assignment \
                will
-     * be saved. The shortcut will then be restored every time the action's 
-     * globalShortcutAllowed flag becomes true.
-     * \e This \e includes \e calling \e setGlobalShortcut()!
+     * be saved. The shortcut will then be restored every time setGlobalShortcut() \
is +     * called with @p loading == Autoloading.
+     *
      * If you actually want to change the global shortcut you have to set
-     * @p loading to NoAutoloading. The new shortcut will be saved again.
+     * @p loading to NoAutoloading. The new shortcut will be automatically saved \
                again.
      *
      * \param shortcut global shortcut(s) to assign. Will be ignored unless \p \
                loading is set to NoAutoloading.
      * \param type the type of shortcut to be set, whether the active shortcut, the \
                default shortcut,
      *             or both (the default).
-     * \param loading load the previous shortcut, which might have been changed by \
                the user (Autoloading, the default)
-     *                 or really set \param shortcut as the new shortcut \
                (NoAutoloading).
-     *
-     * \sa KGlobalAccel
+     * \param loading if Autoloading, assign the global shortcut this action has \
previously had if any. +     *                   That way user preferences and \
changes made to avoid clashes will be conserved. +     *                if \
NoAutoloading the given shortcut will be assigned without looking up old values. +    \
*                   You should only do this if the user wants to change the shortcut \
or if you have +     *                   another very good reason.
+     * \note the default shortcut will never be influenced by autoloading - it will \
be set as given. +     * \sa isGlobalShortcutEnabled()
      * \sa globalShortcut()
      */
     void setGlobalShortcut(const KShortcut& shortcut, ShortcutTypes type =
@@ -365,20 +370,43 @@ public:
     /**
      * Returns true if this action is permitted to have a global shortcut.
      * Defaults to false.
+     * Use isGlobalShortcutEnabled() instead.
      */
-    bool globalShortcutAllowed() const;
+    KDE_DEPRECATED bool globalShortcutAllowed() const;
 
     /**
      * Indicate whether the programmer and/or user may define a global shortcut for \
                this action.
      * Defaults to false. Note that calling setGlobalShortcut() turns this on \
                automatically.
      *
      * \param allowed set to \e true if this action may have a global shortcut, \
                otherwise \e false.
-     * \param loading (see setGlobalShortcut). Ignore this parameter.
+     * \param loading if Autoloading, assign to this action the global shortcut it \
has previously had +     *                if any.
      */
-    void setGlobalShortcutAllowed(bool allowed, GlobalShortcutLoading loading = \
                Autoloading);
-    //^ TODO: NoAutoloading would make little sense in this method, right? We don't \
                have the shortcut to set...
-    // KDE5: remove loading parameter.
+    KDE_DEPRECATED void setGlobalShortcutAllowed(bool allowed, GlobalShortcutLoading \
loading = Autoloading);  
+    /**
+     * Returns true if this action is enabled to have a global shortcut.
+     * Defaults to false.
+     * @see setGlobalShortcutEnabled()
+     */
+    bool isGlobalShortcutEnabled() const;
+
+    /**
+     * Enables this action to have a global shortcut. The action must have a per \
main component unique +     * objectName() to enable cross-application bookeeping. If \
the objectName() is empty this method will +     * return false.
+     * It is mandatory that the objectName() doesn't change once \
isGlobalshortcutEnabled() +     * has become true.
+     * \note KActionCollection::insert(name, action) will set action's objectName to \
name so you often +     * don't have to set an objectName explicitly.
+     */
+    bool enableGlobalShortcut();
+
+    /**
+     * Sets the globalShortcutEnabled property to false and sets the global shortcut \
to an +     * empty shortcut.
+     */
+    void disableGlobalShortcut();
 
     KShapeGesture shapeGesture(ShortcutTypes type = ActiveShortcut) const;
     KRockerGesture rockerGesture(ShortcutTypes type = ActiveShortcut) const;
diff -r fcc9e08c2d7a kdeui/actions/kaction_p.h
--- a/kdeui/actions/kaction_p.h	Thu Feb 28 03:33:48 2008 +0000
+++ b/kdeui/actions/kaction_p.h	Thu Feb 28 09:45:49 2008 +0100
@@ -28,7 +28,7 @@ class KActionPrivate
 {
     public:
         KActionPrivate()
-            : globalShortcutAllowed(false), q(0)
+            : globalShortcutEnabled(false), q(0)
         {
         }
 
@@ -41,7 +41,7 @@ class KActionPrivate
         KShapeGesture shapeGesture, defaultShapeGesture;
         KRockerGesture rockerGesture, defaultRockerGesture;
 
-        bool globalShortcutAllowed;
+        bool globalShortcutEnabled;
         KAction *q;
 };
 
diff -r fcc9e08c2d7a kdeui/shortcuts/kdedglobalaccel.cpp
--- a/kdeui/shortcuts/kdedglobalaccel.cpp	Thu Feb 28 03:33:48 2008 +0000
+++ b/kdeui/shortcuts/kdedglobalaccel.cpp	Thu Feb 28 09:45:49 2008 +0100
@@ -53,21 +53,38 @@ K_PLUGIN_FACTORY(KdedGlobalAccelFactory,
     )
 K_EXPORT_PLUGIN(KdedGlobalAccelFactory("globalaccel"))
 
+struct componentData
+{
+    QString uniqueName;
+    //the name as it would be found in a magazine article about the application,
+    //possibly localized if a localized name exists.
+    QString friendlyName;
+    QHash<QString, actionData *> actions;
+};
+
 struct actionData
 {
 //TODO: clear isPresent when an action's application/mainComponent disappears
-    bool isPresent : 1;
-    bool isDefaultEmpty : 1;
-    QStringList actionId;
+    bool isPresent:1;
+    bool isFresh:1;
+    componentData *parent;
+    QString uniqueName;
+    QString friendlyName; //usually localized
     QList<int> keys;
     QList<int> defaultKeys;
 };
 
-enum IdField
+enum actionIdFields
 {
-    ComponentField = 0,
-    ActionField = 1
+    ComponentUnique = 0,
+    ActionUnique = 1,
+    ComponentFriendly = 2,
+    ActionFriendly = 3
 };
+
+
+//Consider to emit warnings if an actionId does not contain enough elements of that \
turns out +//to be a source of bugs.
 
 
 class KdedGlobalAccelPrivate
@@ -79,7 +96,6 @@ public:
     actionData *findAction(const QStringList &actionId) const;
     actionData *addAction(const QStringList &actionId);
     actionData *takeAction(const QStringList &actionId);
-    QList<actionData *> componentActions(const QString &mainComponentName);
 
     //helpers
     static bool isEmpty(const QList<int>&);
@@ -88,16 +104,15 @@ public:
     KGlobalAccelImpl *impl;
 
     QHash<int, actionData *> keyToAction;
-    QHash<QString, QHash<QString, actionData *> *> mainComponentHashes;
+    QHash<QString, componentData *> mainComponents;
 
     KConfig config;
-    KConfigGroup configGroup;
     QTimer writeoutTimer;
 };
 
 
 KdedGlobalAccelPrivate::KdedGlobalAccelPrivate()
- : config("kglobalshortcutsrc"), configGroup(&config, "KDE Global Shortcuts")
+ : config("kglobalshortcutsrc", KConfig::SimpleConfig)
 {
 }
 
@@ -117,37 +132,41 @@ actionData *KdedGlobalAccelPrivate::find
 {
     if (actionId.count() < 2)
         return 0;
-    QHash<QString, actionData *> *componentHash = \
                mainComponentHashes.value(actionId.at(ComponentField));
-    if (!componentHash)
+    componentData *cd = mainComponents.value(actionId.at(ComponentUnique));
+    if (!cd)
         return 0;
-    return componentHash->value(actionId.at(ActionField));
+    return cd->actions.value(actionId.at(ActionUnique));
 }
 
 
 actionData *KdedGlobalAccelPrivate::addAction(const QStringList &actionId)
 {
-    QHash<QString, actionData *> *componentHash =
-          mainComponentHashes.value(actionId.at(ComponentField));
-    if (!componentHash) {
-        componentHash = new QHash<QString, actionData *>;
-        mainComponentHashes.insert(actionId.at(ComponentField), componentHash);
+    Q_ASSERT(actionId.size() >= 4);
+    componentData *cd = mainComponents.value(actionId.at(ComponentUnique));
+    if (!cd) {
+        cd = new componentData;
+        cd->uniqueName = actionId.at(ComponentUnique);
+        cd->friendlyName = actionId.at(ComponentFriendly);
+        mainComponents.insert(actionId.at(ComponentUnique), cd);
     }
-    Q_ASSERT(!componentHash->value(actionId.at(ActionField)));
+    Q_ASSERT(!cd->actions.value(actionId.at(ActionUnique)));
     actionData *ad = new actionData;
-    ad->actionId = actionId;
-    componentHash->insert(actionId.at(ActionField), ad);
+    ad->parent = cd;
+    ad->uniqueName = actionId.at(ActionUnique);
+    ad->friendlyName = actionId.at(ActionFriendly);
+    cd->actions.insert(actionId.at(ActionUnique), ad);
     return ad;
 }
 
 
 actionData *KdedGlobalAccelPrivate::takeAction(const QStringList &actionId)
 {
-    QHash<QString, actionData *> *componentHash = \
                mainComponentHashes.value(actionId.at(ComponentField));
-    if (!componentHash)
+    componentData *cd = mainComponents.value(actionId.at(ComponentUnique));
+    if (!cd)
         return 0;
-    actionData *ret = componentHash->take(actionId.at(ActionField));
-    if (componentHash->isEmpty())
-        delete mainComponentHashes.take(actionId.at(ComponentField));
+    actionData *ret = cd->actions.take(actionId.at(ActionUnique));
+    if (cd->actions.isEmpty())
+        delete mainComponents.take(actionId.at(ComponentUnique));
     return ret;
 }
 
@@ -179,7 +198,8 @@ QList<int> KdedGlobalAccelPrivate::nonem
 
 
 KdedGlobalAccel::KdedGlobalAccel(QObject* parent, const QList<QVariant>&)
-    : KDEDModule(parent), d(new KdedGlobalAccelPrivate)
+ : KDEDModule(parent),
+   d(new KdedGlobalAccelPrivate)
 {
     qDBusRegisterMetaType<QList<int> >();
 
@@ -206,12 +226,12 @@ KdedGlobalAccel::~KdedGlobalAccel()
 
 QStringList KdedGlobalAccel::allComponents()
 {
-    return d->mainComponentHashes.keys();
+    return d->mainComponents.keys();
 }
 
 QStringList KdedGlobalAccel::allActionsForComponent(const QString& component)
 {
-    return d->mainComponentHashes[component]->keys();
+    return d->mainComponents[component]->actions.keys();
 }
 
 QList<int> KdedGlobalAccel::allKeys()
@@ -231,10 +251,12 @@ QStringList KdedGlobalAccel::allKeysAsSt
 
 QStringList KdedGlobalAccel::actionId(int key)
 {
-    actionData *ad = d->findAction(key);
-    if (ad)
-        return ad->actionId;
-    return QStringList();
+    QStringList ret;
+    if (actionData *ad = d->findAction(key)) {
+        ret.append(ad->parent->uniqueName);
+        ret.append(ad->uniqueName);
+    }
+    return ret;
 }
 
 
@@ -256,47 +278,61 @@ QList<int> KdedGlobalAccel::defaultShort
 }
 
 
+void KdedGlobalAccel::doRegister(const QStringList &actionId)
+{
+    if (actionId.size() < 4) {
+        return;
+    }
+    actionData *ad = d->findAction(actionId);
+    if (!ad) {
+        ad = d->addAction(actionId);
+        //addAction only fills in the names
+        ad->isPresent = false;
+        ad->isFresh = true;
+    }
+}
+
+
 //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)
 {
     //spare the DBus framework some work
-    const bool isDefaultEmpty = (flags & IsDefaultEmpty);
     const bool setPresent = (flags & SetPresent);
     const bool isAutoloading = !(flags & NoAutoloading);
     const bool isDefault = (flags & IsDefault);
 
-    //kDebug() << actionId << keys << "isDefaultEmpty=" << isDefaultEmpty << \
                "setPresent=" << setPresent
-    //         << "isAutoloading=" << isAutoloading << "isDefault=" << isDefault;
+    actionData *ad = d->findAction(actionId);
+    if (!ad) {
+        return QList<int>();
+    }
 
-    actionData *ad = d->findAction(actionId);
+    //default shortcuts cannot clash because they don't do anything
+    if (isDefault) {
+        if (ad->defaultKeys != keys) {
+            ad->defaultKeys = keys;
+            scheduleWriteSettings();
+        }
+        return keys;    //doesn't matter
+    }
 
     //the trivial and common case - synchronize the action from our data and exit
-    bool loadKeys = (isAutoloading && ad);
-    if (loadKeys) {
+    if (isAutoloading && !ad->isFresh) {
         if (!ad->isPresent && setPresent) {
             ad->isPresent = true;
-            foreach (int key, ad->keys)
+            foreach (int key, ad->keys) {
                 if (key != 0) {
                     Q_ASSERT(d->keyToAction.value(key) == ad);
                     d->impl->grabKey(key, true);
                 }
+            }
         }
-        ad->isDefaultEmpty = isDefaultEmpty;
         return ad->keys;
     }
 
     //now we are actually changing the shortcut of the action
 
     QList<int> added = d->nonemptyOnly(keys);
-
-    bool didCreate = false;
-    if (!ad) {
-        didCreate = true;
-        ad = d->addAction(actionId);
-        ad->isPresent = false;
-        //the rest will be initialized below
-    }
 
     //take care of stale keys and remove from added these that remain.
     foreach(int oldKey, ad->keys) {
@@ -312,20 +348,23 @@ QList<int> KdedGlobalAccel::setShortcut(
             }
             if (!remains) {
                 d->keyToAction.remove(oldKey);
-                if (ad->isPresent)
+                if (ad->isPresent) {
                     d->impl->grabKey(oldKey, false);
+                }
             }
         }
     }
 
     //update ad
     //note that ad->keys may still get changed later if conflicts are found
-    ad->isDefaultEmpty = isDefaultEmpty;
     if (setPresent)
         ad->isPresent = true;
-    if (isDefault)
-        ad->defaultKeys = keys;
     ad->keys = keys;
+    //maybe isFresh should really only be set if setPresent, but only two things \
should use !setPresent: +    //- the global shortcuts KCM: very unlikely to catch \
KWin/etc.'s actions in isFresh state +    //- \
KGlobalAccel::stealGlobalShortcutSystemwide(): only applies to actions with shortcuts \
+    //  which can never be fresh if created the usual way +    ad->isFresh = false;
 
     //update keyToAction and find conflicts with other actions
     //this code inherently does the right thing for duplicates in added
@@ -333,7 +372,7 @@ QList<int> KdedGlobalAccel::setShortcut(
         if (!d->keyToAction.contains(added[i])) {
             d->keyToAction.insert(added[i], ad);
         } else {
-            //conflict
+            //clash
             for (int j = 0; j < ad->keys.count(); j++) {
                 if (ad->keys[j] == added[i]) {
                     if (ad->keys.last() == added[i]) {
@@ -348,13 +387,12 @@ QList<int> KdedGlobalAccel::setShortcut(
         }
     }
 
-    if (ad->isPresent)
+    if (ad->isPresent) {
         foreach (int key, added) {
             Q_ASSERT(d->keyToAction.value(key) == ad);
             d->impl->grabKey(key, true);
         }
-
-    scheduleWriteSettings();
+    }
 
     return ad->keys;
 }
@@ -367,8 +405,6 @@ void KdedGlobalAccel::setForeignShortcut
         return;
 
     uint setterFlags = NoAutoloading;
-    if (ad->isDefaultEmpty)
-        setterFlags |= IsDefaultEmpty;
 
     QList<int> oldKeys = ad->keys;
     QList<int> newKeys = setShortcut(actionId, keys, setterFlags);
@@ -402,84 +438,93 @@ void KdedGlobalAccel::scheduleWriteSetti
 //slot
 void KdedGlobalAccel::writeSettings()
 {
-    typedef QHash<QString, actionData*> adHash; //avoid comma in macro arguments
-    foreach (const adHash *const mc, d->mainComponentHashes) {
-        foreach (const actionData *const ad, *mc) {
-            QString confKey = ad->actionId.join("\01");
-            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);
+    foreach (const componentData *const cd, d->mainComponents) {
+        KConfigGroup configGroup(&d->config, cd->uniqueName);
+
+        KConfigGroup friendlyGroup(&configGroup, "Friendly Name");  // :)
+        friendlyGroup.writeEntry("Friendly Name", cd->friendlyName);
+
+        foreach (const actionData *const ad, cd->actions) {
+            if (ad->isFresh) {
+                //no shortcut assignement took place, the action was only registered
+                //(we could still write it out to document its existence, but the \
"fresh" +                //state should be regarded as transitional *only*.)
+                continue;
             }
-            else if (!d->isEmpty(ad->keys))
-                d->configGroup.writeEntry(confKey, stringFromKeys(ad->keys));
-            else
-                d->configGroup.writeEntry(confKey, "none");
+            QStringList entry(stringFromKeys(ad->keys));
+            entry.append(stringFromKeys(ad->defaultKeys));
+            entry.append(ad->friendlyName);
+
+            configGroup.writeEntry(ad->uniqueName, entry);
         }
     }
 
-    d->configGroup.sync();
+    d->config.sync();
 }
 
 
 void KdedGlobalAccel::loadSettings()
 {
-    //TODO: more sanity checks
-    QMap<QString, QString> entries = d->configGroup.entryMap();
-    QString empty;
-    QStringList lActionId(empty);
-    lActionId.append(empty);
+    QStringList lActionId;
+    for (int i = 0; i < 4; i++) {
+        lActionId.append(QString());
+    }
 
-    QMap<QString, QString>::const_iterator it;
-    for (it = entries.constBegin(); it != entries.constEnd(); ++it) {
-        //cut at the first occurrence *only*, so no split('\01')
-        int snip = it.key().indexOf('\01');
-        //TODO: definitely more sanity checks, like bounds check of (snip + 1) :)
-        lActionId[ComponentField] = it.key().left(snip);
-        lActionId[ActionField] = it.key().mid(snip + 1);
-        QList<int> lKeys = keysFromString(it.value());
+    foreach (const QString &groupName, d->config.groupList()) {
+        KConfigGroup configGroup(&d->config, groupName);
+        lActionId[ComponentUnique] = groupName;
 
-        actionData *ad = d->addAction(lActionId);
-        ad->keys = lKeys;
-        ad->isPresent = false;
-        //If we loaded an empty action, that action must have been saved
-        //because it was *not* empty by default. This boolean does not propagate
-        //out of this class, so it's ok to mess with it as we like.
-        ad->isDefaultEmpty = false;
+        KConfigGroup friendlyGroup(&configGroup, "Friendly Name");
+        lActionId[ComponentFriendly] = friendlyGroup.readEntry("Friendly Name");
 
-        foreach (int key, lKeys)
-            if (key != 0)
-                d->keyToAction.insert(key, ad);
+        foreach (const QString &confKey, configGroup.keyList()) {
+            QStringList entry = configGroup.readEntry(confKey, QStringList());
+            if (entry.size() != 3) {
+                continue;
+            }
+            lActionId[ActionUnique] = confKey;
+            lActionId[ActionFriendly] = entry[2];
+
+            actionData *ad = d->addAction(lActionId);
+            ad->keys = keysFromString(entry[0]);
+            ad->defaultKeys = keysFromString(entry[1]);
+            ad->isPresent = false;
+            ad->isFresh = false;
+    
+            foreach (int key, ad->keys) {
+                if (key != 0) {
+                    d->keyToAction.insert(key, ad);
+                }
+            }
+        }
     }
 }
-
 
 QList<int> KdedGlobalAccel::keysFromString(const QString &str)
 {
     QList<int> ret;
-    if (str == "none")
+    if (str == "none") {
         return ret;
-
-    QStringList strList = str.split('\01');
-    foreach (const QString &s, strList)
+    }
+    QStringList strList = str.split(' ');
+    foreach (const QString &s, strList) {
         ret.append(QKeySequence(s)[0]);
-
+    }
     return ret;
 }
 
 
 QString KdedGlobalAccel::stringFromKeys(const QList<int> &keys)
 {
-    //the special output "none" is generated at the caller
+    if (keys.isEmpty()) {
+        return "none";
+    }
     QString ret;
     foreach (int key, keys) {
         ret.append(QKeySequence(key).toString());
-        ret.append('\01');
+        ret.append(' ');
     }
-    //this is safe if the index is out of bounds
-    ret.truncate(ret.length() - 1);
-
+    ret.chop(1);
     return ret;
 }
 
@@ -490,7 +535,10 @@ bool KdedGlobalAccel::keyPressed(int key
     if (!ad || !ad->isPresent)
         return false;
 
-    QStringList data = ad->actionId;
+    QStringList data(ad->parent->uniqueName);
+    data.append(ad->uniqueName);
+    data.append(ad->parent->friendlyName);
+    data.append(ad->friendlyName);
 #ifdef Q_WS_X11
     // pass X11 timestamp
     data.append(QString::number(QX11Info::appTime()));
diff -r fcc9e08c2d7a kdeui/shortcuts/kdedglobalaccel.h
--- a/kdeui/shortcuts/kdedglobalaccel.h	Thu Feb 28 03:33:48 2008 +0000
+++ b/kdeui/shortcuts/kdedglobalaccel.h	Thu Feb 28 09:45:49 2008 +0100
@@ -19,6 +19,20 @@
     Boston, MA 02110-1301, USA.
 */
 
+/*
+data per main component:
+-unique name
+-i18n name
+-(KActions)
+
+data per KAction:
+-unique name
+-i18n name
+-shortcut
+-default shortcut (special purpose API for KCMs)
+
+ */
+
 #ifndef KDEDGLOBALACCEL_H
 #define KDEDGLOBALACCEL_H
 
@@ -35,7 +49,6 @@ public:
 public:
     enum SetShortcutFlags
     {
-        IsDefaultEmpty = 1,
         SetPresent = 2,
         NoAutoloading = 4,
         IsDefault = 8
@@ -44,6 +57,7 @@ public:
     KdedGlobalAccel(QObject*, const QList<QVariant>&);
     ~KdedGlobalAccel();
 
+//All of the following public methods and signals are part of the DBus interface
     QStringList allComponents();
     QStringList allActionsForComponent(const QString& component);
 
@@ -64,13 +78,25 @@ public:
     //conflict resolution but won't trigger.
     void setInactive(const QStringList &actionId);
 
+    void doRegister(const QStringList &actionId);
+
     //called by the implementation to inform us about key presses
-    //returns true when the key was handled
+    //returns true if the key was handled
     bool keyPressed(int keyQt);
 
     //not implementetd ATM, TODO: why does it even exist?
     void regrabKeys() {}
-//the DBus interface
+
+////////////////////////////////////////
+    //should be called from KAction::setGlobalShortcut() once after application \
startup. +    //as a little hack, to lose the data of a component permanently you can \
first "unset" +    //all shortcuts and then set the component name to the empty \
string. +    void setComponentCommonName(const QString &uniqueName, const QString \
&commonName); +
+    QString componentCommonName(const QString &uniqueName);
+    QString actionCommonName(const QString &uniqueName);
+
+
 Q_SIGNALS:
     void invokeAction(const QStringList &actionId);
     void yourShortcutGotChanged(const QStringList &actionId, const QList<int> \
                &newKeys);
diff -r fcc9e08c2d7a kdeui/shortcuts/kdedglobalaccel_adaptor.h
--- a/kdeui/shortcuts/kdedglobalaccel_adaptor.h	Thu Feb 28 03:33:48 2008 +0000
+++ b/kdeui/shortcuts/kdedglobalaccel_adaptor.h	Thu Feb 28 09:45:49 2008 +0100
@@ -82,6 +82,8 @@ public Q_SLOTS:
     //until woken up again by calling setShortcut().
     inline Q_NOREPLY void setInactive(const QStringList &actionId)
         { return p()->setInactive(actionId); }
+    inline Q_NOREPLY void doRegister(const QStringList &actionId)
+        { return p()->doRegister(actionId); }
 
 Q_SIGNALS:
     void invokeAction(const QStringList &actionId);
diff -r fcc9e08c2d7a kdeui/shortcuts/kdedglobalaccel_interface.h
--- a/kdeui/shortcuts/kdedglobalaccel_interface.h	Thu Feb 28 03:33:48 2008 +0000
+++ b/kdeui/shortcuts/kdedglobalaccel_interface.h	Thu Feb 28 09:45:49 2008 +0100
@@ -98,6 +98,14 @@ public Q_SLOTS: // METHODS
                              argumentList);
     }
 
+    Q_NOREPLY void doRegister(const QStringList &actionId)
+    {
+        QList<QVariant> argumentList;
+        argumentList << qVariantFromValue(actionId);
+        callWithArgumentList(QDBus::NoBlock, QLatin1String("doRegister"),
+                             argumentList);
+    }
+
 //The signals part seems to work by pure black magic, i.e. clever hacks
 //with the Qt meta-object / signals & slots system. Maybe.
 Q_SIGNALS: // SIGNALS
diff -r fcc9e08c2d7a kdeui/shortcuts/kglobalaccel.cpp
--- a/kdeui/shortcuts/kglobalaccel.cpp	Thu Feb 28 03:33:48 2008 +0000
+++ b/kdeui/shortcuts/kglobalaccel.cpp	Thu Feb 28 09:45:49 2008 +0100
@@ -49,17 +49,25 @@
 #include <kdebug.h>
 #include <klocale.h>
 #include <ktoolinvocation.h>
+#include <kcomponentdata.h>
 #include <kconfig.h>
 #include <kconfiggroup.h>
 #include <kglobal.h>
 #include "kaction.h"
 #include "kaction_p.h"
-#include "kactioncollection.h"
 #include "kmessagebox.h"
 #include "kshortcut.h"
 
 
-//TODO what was the problem that got fixed recently in the old version? - forward \
port if necessary +//### copied over from kdedglobalaccel.cpp to avoid more includes
+enum actionIdFields
+{
+    ComponentUnique = 0,
+    ActionUnique = 1,
+    ComponentFriendly = 2,
+    ActionFriendly = 3
+};
+
 
 KGlobalAccelPrivate::KGlobalAccelPrivate(KGlobalAccel* q)
      : isUsingForeignComponentName(false),
@@ -75,6 +83,7 @@ KGlobalAccelPrivate::KGlobalAccelPrivate
             q, SLOT(_k_serviceOwnerChanged(QString,QString,QString)));
 }
 
+
 KGlobalAccel::KGlobalAccel()
     : d(new KGlobalAccelPrivate(this))
 {
@@ -101,6 +110,7 @@ bool KGlobalAccel::isEnabled() const
 {
     return d->enabled;
 }
+
 
 void KGlobalAccel::setEnabled( bool enabled )
 {
@@ -135,77 +145,88 @@ KGlobalAccel *KGlobalAccel::self( )
     return s_instance;
 }
 
-QList<int> KGlobalAccelPrivate::updateGlobalShortcutInKded(KAction* action, const \
                QStringList& actionId, uint flags, uint initialSetterFlags)
-{
-    uint setterFlags = initialSetterFlags;
-    const KShortcut defaultShortcut = \
                action->globalShortcut(KAction::DefaultShortcut);
-    const KShortcut activeShortcut = action->globalShortcut();
-    if (flags & KAction::NoAutoloading)
-        setterFlags |= KdedGlobalAccel::NoAutoloading;
-    if (defaultShortcut.isEmpty())
-        setterFlags |= KdedGlobalAccel::IsDefaultEmpty;
-    if (defaultShortcut == activeShortcut)
-        setterFlags |= KdedGlobalAccel::IsDefault;
 
-    const QList<int> result = iface.setShortcut(actionId,
-                                                intListFromShortcut(activeShortcut),
-                                                setterFlags);
-    const KShortcut scResult(shortcutFromIntList(result));
-
-    if (scResult != activeShortcut)
-        action->d->setActiveGlobalShortcutNoEnable(scResult);
-    return result;
-}
-
-void KGlobalAccelPrivate::updateGlobalShortcutAllowed(KAction *action, uint flags)
+void KGlobalAccelPrivate::doRegister(KAction *action)
 {
     if (!action)
         return;
 
-    bool oldEnabled = actionToName.contains(action);
-    bool newEnabled = action->globalShortcutAllowed();
-
-    if (oldEnabled == newEnabled)
+    const bool isRegistered = actionToName.contains(action);
+    if (isRegistered || action->objectName().isEmpty())
         return;
 
-    if (action->text().isEmpty())
-        return;
-    QStringList actionId(mainComponentName);
-    actionId.append(action->text());
-    //TODO: what about i18ned names?
+    QStringList actionId = makeActionId(action);
 
-    if (!oldEnabled && newEnabled) {
-        // action is now enabled
-        nameToAction.insert(actionId.at(1), action);
-        actionToName.insert(action, actionId.at(1));
-        updateGlobalShortcutInKded(action, actionId, flags, \
                KdedGlobalAccel::SetPresent);
-    }
-    else if (oldEnabled && !newEnabled) {
-        // action is now disabled
-        nameToAction.remove(actionToName.take(action));
-        iface.setInactive(actionId);
-    }
+    nameToAction.insert(actionId.at(ActionUnique), action);
+    actionToName.insert(action, actionId.at(ActionUnique));
+    iface.doRegister(actionId);
 }
 
 
-void KGlobalAccelPrivate::updateGlobalShortcut(KAction *action, uint flags)
+void KGlobalAccelPrivate::setInactive(KAction *action)
 {
     if (!action)
         return;
 
-    if (action->text().isEmpty())
+    const bool isRegistered = actionToName.contains(action);
+    if (!isRegistered || action->objectName().isEmpty())
         return;
-    QStringList actionId(mainComponentName);
-    actionId.append(action->text());
-    //TODO: what about i18ned names?
 
-    const QList<int> result = updateGlobalShortcutInKded(action, actionId, flags, \
0); +    QStringList actionId = makeActionId(action);
 
-    //We might be able to avoid that call sometimes, but it's neither worth the \
                effort nor
-    //the bytes to determine the cases where it's safe to avoid it.
-    if (isUsingForeignComponentName) {
-        iface.setForeignShortcut(actionId, result);
+    nameToAction.remove(actionToName.take(action));
+    iface.setInactive(actionId);
+}
+
+
+void KGlobalAccelPrivate::updateGlobalShortcut(KAction *action, uint flags)
+{
+    if (!action || action->objectName().isEmpty()) {
+        return;
     }
+
+    QStringList actionId = makeActionId(action);
+    const KShortcut activeShortcut = action->globalShortcut();
+    const KShortcut defaultShortcut = \
action->globalShortcut(KAction::DefaultShortcut); +
+    uint setterFlags = 0;
+    if (flags & KAction::NoAutoloading) {
+        setterFlags |= KdedGlobalAccel::NoAutoloading;
+    }
+
+    if (flags & KAction::ActiveShortcut) {
+        uint activeSetterFlags = setterFlags;
+        if (!isUsingForeignComponentName) {
+            activeSetterFlags |= KdedGlobalAccel::SetPresent;
+        }
+
+        const QList<int> result = iface.setShortcut(actionId,
+                                                    \
intListFromShortcut(activeShortcut), +                                                \
activeSetterFlags); +        const KShortcut scResult(shortcutFromIntList(result));
+        if (isUsingForeignComponentName) {
+            iface.setForeignShortcut(actionId, result);
+        } else if (scResult != activeShortcut) {
+            action->d->setActiveGlobalShortcutNoEnable(scResult);
+        }
+    }
+
+    if (flags & KAction::DefaultShortcut) {
+        iface.setShortcut(actionId, intListFromShortcut(defaultShortcut),
+                          setterFlags | KdedGlobalAccel::IsDefault);
+    }
+}
+
+
+QStringList KGlobalAccelPrivate::makeActionId(const KAction *action)
+{
+    Q_ASSERT(!mainComponentName.isEmpty());
+    Q_ASSERT(!action->objectName().isEmpty());
+    QStringList ret(mainComponentName);
+    ret.append(action->objectName());
+    ret.append(mainComponentName); //### find (or introduce) a way to get a \
"friendly" component name +    ret.append(action->text());
+    return ret;
 }
 
 
@@ -235,13 +256,11 @@ void KGlobalAccelPrivate::_k_invokeActio
 {
     //TODO: can we make it so that we don't have to check the mainComponentName? \
                (i.e. targeted signals)
     // Well, how about making the full QStringList the key in nameToAction?
-    if (actionId.at(0) != mainComponentName || isUsingForeignComponentName)
+    if (actionId.at(ComponentUnique) != mainComponentName || \
isUsingForeignComponentName)  return;
 
-    KAction *action = nameToAction.value(actionId.at(1));
-    if (!action)
-        return;
-    if (!action->isEnabled())
+    KAction *action = nameToAction.value(actionId.at(ActionUnique));
+    if (!action || !action->isEnabled())
         return;
 
 #ifdef Q_WS_X11
@@ -249,11 +268,11 @@ void KGlobalAccelPrivate::_k_invokeActio
     // TODO The 100%-correct solution should probably be handling this action
     // in the proper place in relation to the X events queue in order to avoid
     // the possibility of wrong ordering of user events.
-    Time timestamp = actionId.at( 2 ).toULong();
-    if( NET::timestampCompare( timestamp, QX11Info::appTime()) > 0 )
-        QX11Info::setAppTime( timestamp );
-    if( NET::timestampCompare( timestamp, QX11Info::appUserTime()) > 0 )
-        QX11Info::setAppUserTime( timestamp );
+    Time timestamp = actionId.at(4).toULong();
+    if( NET::timestampCompare(timestamp, QX11Info::appTime()) > 0)
+        QX11Info::setAppTime(timestamp);
+    if( NET::timestampCompare(timestamp, QX11Info::appUserTime()) > 0)
+        QX11Info::setAppUserTime(timestamp);
 #endif
 
     action->trigger();
@@ -262,7 +281,7 @@ void KGlobalAccelPrivate::_k_shortcutGot
 void KGlobalAccelPrivate::_k_shortcutGotChanged(const QStringList &actionId,
                                                 const QList<int> &keys)
 {
-    KAction *action = nameToAction.value(actionId.at(1));
+    KAction *action = nameToAction.value(actionId.at(ActionUnique));
     if (!action)
         return;
 
@@ -293,7 +312,8 @@ void KGlobalAccelPrivate::reRegisterAll(
     nameToAction.clear();
     actionToName.clear();
     foreach(KAction *const action, allActions) {
-        updateGlobalShortcutAllowed(action, 0/*flags*/);
+        doRegister(action);
+        updateGlobalShortcut(action, KAction::Autoloading | \
KAction::ActiveShortcut);  }
 }
 
@@ -308,11 +328,15 @@ QStringList KGlobalAccel::findActionName
 //static
 bool KGlobalAccel::promptStealShortcutSystemwide(QWidget *parent, const QStringList \
&actionIdentifier, const QKeySequence &seq)  {
+    if (actionIdentifier.size() < 4) {
+        return false;
+    }
     QString title = i18n("Conflict with Global Shortcut");
     QString message = i18n("The '%1' key combination has already been allocated "
                            "to the global action \"%2\" in %3.\n"
                            "Do you want to reassign it from that action to the \
                current one?",
-                           seq.toString(), actionIdentifier.at(1), \
actionIdentifier.at(0)); +                           seq.toString(), \
actionIdentifier.at(ActionFriendly), +                           \
actionIdentifier.at(ComponentFriendly));  
     return KMessageBox::warningContinueCancel(parent, message, title, \
KGuiItem(i18n("Reassign")))  == KMessageBox::Continue;
@@ -324,7 +348,7 @@ void KGlobalAccel::stealShortcutSystemwi
 {
     //get the shortcut, remove seq, and set the new shorctut
     const QStringList actionId = self()->d->iface.action(seq[0]);
-    if (actionId.size() < 2) // not a global shortcut
+    if (actionId.size() < 4) // not a global shortcut
         return;
     QList<int> sc = self()->d->iface.shortcut(actionId);
 
diff -r fcc9e08c2d7a kdeui/shortcuts/kglobalaccel_p.h
--- a/kdeui/shortcuts/kglobalaccel_p.h	Thu Feb 28 03:33:48 2008 +0000
+++ b/kdeui/shortcuts/kglobalaccel_p.h	Thu Feb 28 09:45:49 2008 +0100
@@ -37,19 +37,19 @@ public:
 
     ///Propagate any shortcut changes to the KDED module that does the bookkeeping
     ///and the key grabbing.
-    ///If this is called with an action that has an empty active global shortcut and
-    ///an empty default shortcut, the record of that action will be deleted.
     void updateGlobalShortcut(KAction *action, /*KAction::ShortcutTypes*/uint \
flags);  
-    ///Register or unregister the action in this class, and notify the KDED module
-    void updateGlobalShortcutAllowed(KAction *action, /*KAction::ShortcutTypes*/uint \
flags); +    ///Register the action in this class and in the KDED module
+    void doRegister(KAction *action);   //"register" is a C keyword :p
+    ///"Forget" the action in this class and mark it as not present in the KDED \
module +    void setInactive(KAction *action);
 
-    /// Helper method for the above two, takes care of the actual call to kded
-    QList<int> updateGlobalShortcutInKded(KAction* action, const QStringList& \
                actionId, uint flags, uint initialSetterFlags);
-
+    //"private" helpers
+    QStringList makeActionId(const KAction *action);
     QList<int> intListFromShortcut(const KShortcut &cut);
     KShortcut shortcutFromIntList(const QList<int> &list);
 
+    //private slot implementations
     void _k_invokeAction(const QStringList&);
     void _k_shortcutGotChanged(const QStringList&, const QList<int>&);
     void _k_serviceOwnerChanged(const QString& name, const QString& oldOwner, const \
QString& newOwner);



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

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