Refactor ActionContainer internals.

Make groups explicit instead of magic integer stuff.
That also makes group ordering explicit instead of magically working
(the way we happened to use it and the way unique id manager is
implemented).

4 files changed, 112 insertions, 145 deletions

diff --git a/src/plugins/coreplugin/actionmanager/actioncontainer.cpp b/src/plugins/coreplugin/actionmanager/actioncontainer.cpp
index f47313f569..31e68bd70d 100644
--- a/src/plugins/coreplugin/actionmanager/actioncontainer.cpp
+++ b/src/plugins/coreplugin/actionmanager/actioncontainer.cpp
@@ -39,6 +39,8 @@
 #include "coreconstants.h"
 #include "uniqueidmanager.h"
 
+#include <utils/qtcassert.h>
+
 #include <QtCore/QDebug>
 #include <QtCore/QTimer>
 #include <QtGui/QAction>
@@ -165,6 +167,9 @@ using namespace Core::Internal;
 ActionContainerPrivate::ActionContainerPrivate(int id)
     : m_onAllDisabledBehavior(Disable), m_id(id), m_updateRequested(false)
 {
+    appendGroup(QLatin1String(Constants::G_DEFAULT_ONE));
+    appendGroup(QLatin1String(Constants::G_DEFAULT_TWO));
+    appendGroup(QLatin1String(Constants::G_DEFAULT_THREE));
     scheduleUpdate();
 }
 
@@ -178,144 +183,112 @@ ActionContainer::OnAllDisabledBehavior ActionContainerPrivate::onAllDisabledBeha
     return m_onAllDisabledBehavior;
 }
 
-void ActionContainerPrivate::appendGroup(const QString &group)
-{
-    int gid = UniqueIDManager::instance()->uniqueIdentifier(group);
-    m_groups << gid;
-}
-
-QAction *ActionContainerPrivate::insertLocation(const QString &group) const
+void ActionContainerPrivate::appendGroup(const QString &groupId)
 {
-    int grpid = UniqueIDManager::instance()->uniqueIdentifier(group);
-    int prevKey = 0;
-    int pos = ((grpid << 16) | 0xFFFF);
-    return beforeAction(pos, &prevKey);
+    m_groups.append(Group(groupId));
 }
 
-void ActionContainerPrivate::addAction(Command *action, const QString &group)
+QList<Group>::const_iterator ActionContainerPrivate::findGroup(const QString &groupId) const
 {
-    if (!canAddAction(action))
-        return;
-
-    ActionManagerPrivate *am = ActionManagerPrivate::instance();
-    UniqueIDManager *idmanager = UniqueIDManager::instance();
-    int grpid = idmanager->uniqueIdentifier(Constants::G_DEFAULT_TWO);
-    if (!group.isEmpty())
-        grpid = idmanager->uniqueIdentifier(group);
-    if (!m_groups.contains(grpid) && !am->defaultGroups().contains(grpid))
-        qWarning() << "*** addAction(): Unknown group: " << group;
-    int pos = ((grpid << 16) | 0xFFFF);
-    addActionInternal(action, pos);
+    QList<Group>::const_iterator it = m_groups.constBegin();
+    while (it != m_groups.constEnd()) {
+        if (it->id == groupId)
+            break;
+        ++it;
+    }
+    return it;
 }
 
-void ActionContainerPrivate::addMenu(ActionContainer *menu, const QString &group)
-{
-    ActionContainerPrivate *container = static_cast<ActionContainerPrivate *>(menu);
-    if (!container->canBeAddedToMenu())
-        return;
-
-    ActionManagerPrivate *am = ActionManagerPrivate::instance();
-    UniqueIDManager *idmanager = UniqueIDManager::instance();
-    int grpid = idmanager->uniqueIdentifier(Constants::G_DEFAULT_TWO);
-    if (!group.isEmpty())
-        grpid = idmanager->uniqueIdentifier(group);
-    if (!m_groups.contains(grpid) && !am->defaultGroups().contains(grpid))
-        qWarning() << "*** addMenu(): Unknown group: " << group;
-    int pos = ((grpid << 16) | 0xFFFF);
-    addMenuInternal(menu, pos);
-}
 
-int ActionContainerPrivate::id() const
+QAction *ActionContainerPrivate::insertLocation(const QString &groupId) const
 {
-    return m_id;
+    QList<Group>::const_iterator it = findGroup(groupId);
+    QTC_ASSERT(it != m_groups.constEnd(), return 0);
+    return insertLocation(it);
 }
 
-QMenu *ActionContainerPrivate::menu() const
-{
-    return 0;
-}
-
-QMenuBar *ActionContainerPrivate::menuBar() const
+QAction *ActionContainerPrivate::insertLocation(QList<Group>::const_iterator group) const
 {
+    if (group == m_groups.constEnd())
+        return 0;
+    ++group;
+    while (group != m_groups.constEnd()) {
+        if (!group->items.isEmpty()) {
+            QObject *item = group->items.first();
+            if (Command *cmd = qobject_cast<Command *>(item)) {
+                return cmd->action();
+            } else if (ActionContainer *container = qobject_cast<ActionContainer *>(item)) {
+                if (container->menu())
+                    return container->menu()->menuAction();
+            }
+            QTC_ASSERT(false, return 0);
+        }
+        ++group;
+    }
     return 0;
 }
 
-bool ActionContainerPrivate::canAddAction(Command *action) const
+void ActionContainerPrivate::addAction(Command *command, const QString &groupId)
 {
-    return (action->action() != 0);
-}
+    if (!canAddAction(command))
+        return;
 
-void ActionContainerPrivate::addActionInternal(Command *action, int pos)
-{
-    Action *a = static_cast<Action *>(action);
+    QString actualGroupId;
+    if (groupId.isEmpty())
+        actualGroupId = QLatin1String(Constants::G_DEFAULT_TWO);
+    else
+        actualGroupId = groupId;
 
-    int prevKey = 0;
-    QAction *ba = beforeAction(pos, &prevKey);
-    pos = calcPosition(pos, prevKey);
+    QList<Group>::const_iterator groupIt = findGroup(actualGroupId);
+    QTC_ASSERT(groupIt != m_groups.constEnd(), return);
+    QAction *beforeAction = insertLocation(groupIt);
+    m_groups[groupIt-m_groups.constBegin()].items.append(command);
 
-    m_commands.append(action);
-    m_posmap.insert(pos, action->id());
-    connect(action, SIGNAL(activeStateChanged()), this, SLOT(scheduleUpdate()));
-    insertAction(ba, a->action());
+    connect(command, SIGNAL(activeStateChanged()), this, SLOT(scheduleUpdate()));
+    insertAction(beforeAction, command->action());
     scheduleUpdate();
 }
 
-void ActionContainerPrivate::addMenuInternal(ActionContainer *menu, int pos)
+void ActionContainerPrivate::addMenu(ActionContainer *menu, const QString &groupId)
 {
-    MenuActionContainer *mc = static_cast<MenuActionContainer *>(menu);
+    ActionContainerPrivate *containerPrivate = static_cast<ActionContainerPrivate *>(menu);
+    if (!containerPrivate->canBeAddedToMenu())
+        return;
+    MenuActionContainer *container = static_cast<MenuActionContainer *>(containerPrivate);
+
+    QString actualGroupId;
+    if (groupId.isEmpty())
+        actualGroupId = QLatin1String(Constants::G_DEFAULT_TWO);
+    else
+        actualGroupId = groupId;
 
-    int prevKey = 0;
-    QAction *ba = beforeAction(pos, &prevKey);
-    pos = calcPosition(pos, prevKey);
+    QList<Group>::const_iterator groupIt = findGroup(actualGroupId);
+    QTC_ASSERT(groupIt != m_groups.constEnd(), return);
+    QAction *beforeAction = insertLocation(groupIt);
+    m_groups[groupIt-m_groups.constBegin()].items.append(menu);
 
-    m_subContainers.append(menu);
-    m_posmap.insert(pos, menu->id());
-    insertMenu(ba, mc->menu());
+    insertMenu(beforeAction, container->menu());
     scheduleUpdate();
 }
 
-QAction *ActionContainerPrivate::beforeAction(int pos, int *prevKey) const
+int ActionContainerPrivate::id() const
 {
-    ActionManagerPrivate *am = ActionManagerPrivate::instance();
-
-    int baId = -1;
-
-    (*prevKey) = -1;
-
-    QMap<int, int>::const_iterator i = m_posmap.constBegin();
-    while (i != m_posmap.constEnd()) {
-        if (i.key() > pos) {
-            baId = i.value();
-            break;
-        }
-        (*prevKey) = i.key();
-        ++i;
-    }
-
-    if (baId == -1)
-        return 0;
-
-    if (Command *cmd = am->command(baId))
-        return cmd->action();
-    if (ActionContainer *container = am->actionContainer(baId))
-        if (QMenu *menu = container->menu())
-            return menu->menuAction();
+    return m_id;
+}
 
+QMenu *ActionContainerPrivate::menu() const
+{
     return 0;
 }
 
-int ActionContainerPrivate::calcPosition(int pos, int prevKey) const
+QMenuBar *ActionContainerPrivate::menuBar() const
 {
-    int grp = (pos & 0xFFFF0000);
-    if (prevKey == -1)
-        return grp;
-
-    int prevgrp = (prevKey & 0xFFFF0000);
-
-    if (grp != prevgrp)
-        return grp;
+    return 0;
+}
 
-    return grp + (prevKey & 0xFFFF) + 10;
+bool ActionContainerPrivate::canAddAction(Command *action) const
+{
+    return (action->action() != 0);
 }
 
 void ActionContainerPrivate::scheduleUpdate()
@@ -378,25 +351,32 @@ bool MenuActionContainer::updateInternal()
     bool hasitems = false;
     QList<QAction *> actions = m_menu->actions();
 
-    foreach (ActionContainer *container, subContainers()) {
-        actions.removeAll(container->menu()->menuAction());
-        if (container == this) {
-            qWarning() << Q_FUNC_INFO << "container" << (this->menu() ? this->menu()->title() : "") <<  "contains itself as subcontainer";
-            continue;
-        }
-        if (qobject_cast<ActionContainerPrivate*>(container)->updateInternal()) {
-            hasitems = true;
-            break;
-        }
-    }
-    if (!hasitems) {
-        foreach (Command *command, commands()) {
-            actions.removeAll(command->action());
-            if (command->isActive()) {
-                hasitems = true;
-                break;
+    QListIterator<Group> it(m_groups);
+    while (it.hasNext()) {
+        const Group &group = it.next();
+        foreach (QObject *item, group.items) {
+            if (ActionContainerPrivate *container = qobject_cast<ActionContainerPrivate*>(item)) {
+                actions.removeAll(container->menu()->menuAction());
+                if (container == this) {
+                    qWarning() << Q_FUNC_INFO << "container" << (this->menu() ? this->menu()->title() : "") <<  "contains itself as subcontainer";
+                    continue;
+                }
+                if (container->updateInternal()) {
+                    hasitems = true;
+                    break;
+                }
+            } else if (Command *command = qobject_cast<Command *>(item)) {
+                actions.removeAll(command->action());
+                if (command->isActive()) {
+                    hasitems = true;
+                    break;
+                }
+            } else {
+                QTC_ASSERT(false, continue);
             }
         }
+        if (hasitems)
+            break;
     }
     if (!hasitems) {
         // look if there were actions added that we don't control and check if they are enabled
diff --git a/src/plugins/coreplugin/actionmanager/actioncontainer_p.h b/src/plugins/coreplugin/actionmanager/actioncontainer_p.h
index 6beae138ce..8d313dc98e 100644
--- a/src/plugins/coreplugin/actionmanager/actioncontainer_p.h
+++ b/src/plugins/coreplugin/actionmanager/actioncontainer_p.h
@@ -42,6 +42,12 @@
 namespace Core {
 namespace Internal {
 
+struct Group {
+    Group(const QString &id) : id(id) {}
+    QString id;
+    QList<QObject *> items; // Command * or ActionContainer *
+};
+
 class ActionContainerPrivate : public Core::ActionContainer
 {
     Q_OBJECT
@@ -54,7 +60,7 @@ public:
     ActionContainer::OnAllDisabledBehavior onAllDisabledBehavior() const;
 
     QAction *insertLocation(const QString &group) const;
-    void appendGroup(const QString &group);
+    void appendGroup(const QString &id);
     void addAction(Command *action, const QString &group = QString());
     void addMenu(ActionContainer *menu, const QString &group = QString());
 
@@ -66,9 +72,6 @@ public:
     virtual void insertAction(QAction *before, QAction *action) = 0;
     virtual void insertMenu(QAction *before, QMenu *menu) = 0;
 
-    QList<Command *> commands() const { return m_commands; }
-    QList<ActionContainer *> subContainers() const { return m_subContainers; }
-
     virtual bool updateInternal() = 0;
 
 protected:
@@ -76,23 +79,19 @@ protected:
     bool canAddMenu(ActionContainer *menu) const;
     virtual bool canBeAddedToMenu() const = 0;
 
-    void addActionInternal(Command *action, int pos);
-    void addMenuInternal(ActionContainer *menu, int pos);
+    // groupId --> list of Command* and ActionContainer*
+    QList<Group> m_groups;
 
 private slots:
     void scheduleUpdate();
     void update();
 
 private:
-    QAction *beforeAction(int pos, int *prevKey) const;
-    int calcPosition(int pos, int prevKey) const;
+    QList<Group>::const_iterator findGroup(const QString &groupId) const;
+    QAction *insertLocation(QList<Group>::const_iterator group) const;
 
-    QList<int> m_groups;
     OnAllDisabledBehavior m_onAllDisabledBehavior;
     int m_id;
-    QMap<int, int> m_posmap;
-    QList<ActionContainer *> m_subContainers;
-    QList<Command *> m_commands;
     bool m_updateRequested;
 };
 
diff --git a/src/plugins/coreplugin/actionmanager/actionmanager.cpp b/src/plugins/coreplugin/actionmanager/actionmanager.cpp
index 5f17c3eceb..e28f44abbf 100644
--- a/src/plugins/coreplugin/actionmanager/actionmanager.cpp
+++ b/src/plugins/coreplugin/actionmanager/actionmanager.cpp
@@ -242,12 +242,7 @@ ActionManagerPrivate::ActionManagerPrivate(MainWindow *mainWnd)
   : ActionManager(mainWnd),
     m_mainWnd(mainWnd)
 {
-    UniqueIDManager *uidmgr = UniqueIDManager::instance();
-    m_defaultGroups << uidmgr->uniqueIdentifier(Constants::G_DEFAULT_ONE);
-    m_defaultGroups << uidmgr->uniqueIdentifier(Constants::G_DEFAULT_TWO);
-    m_defaultGroups << uidmgr->uniqueIdentifier(Constants::G_DEFAULT_THREE);
     m_instance = this;
-
 }
 
 ActionManagerPrivate::~ActionManagerPrivate()
@@ -261,11 +256,6 @@ ActionManagerPrivate *ActionManagerPrivate::instance()
     return m_instance;
 }
 
-QList<int> ActionManagerPrivate::defaultGroups() const
-{
-    return m_defaultGroups;
-}
-
 QList<Command *> ActionManagerPrivate::commands() const
 {
     // transform list of CommandPrivate into list of Command
diff --git a/src/plugins/coreplugin/actionmanager/actionmanager_p.h b/src/plugins/coreplugin/actionmanager/actionmanager_p.h
index 4e1304965b..4b953a5a9b 100644
--- a/src/plugins/coreplugin/actionmanager/actionmanager_p.h
+++ b/src/plugins/coreplugin/actionmanager/actionmanager_p.h
@@ -69,7 +69,6 @@ public:
     static ActionManagerPrivate *instance();
 
     void saveSettings(QSettings *settings);
-    QList<int> defaultGroups() const;
 
     QList<Command *> commands() const;
 
@@ -98,7 +97,6 @@ private:
     Action *overridableAction(const Id &id);
 
     static ActionManagerPrivate *m_instance;
-    QList<int> m_defaultGroups;
 
     typedef QHash<int, CommandPrivate *> IdCmdMap;
     IdCmdMap m_idCmdMap;