ProjectExplorer: Add KitAspect convenience

.. for data to be displayed in a QComboBox.
Make use of it for Qt versions, debuggers and CMake tools.

Change-Id: I255f86c97fe30b43c1284842e7f9e5052d098946
Reviewed-by: hjk <hjk@qt.io>

src/plugins/cmakeprojectmanager/cmakekitaspect.cpp

@@ -187,72 +187,31 @@ class CMakeKitAspectImpl final : public KitAspect
 {
 public:
     CMakeKitAspectImpl(Kit *kit, const KitAspectFactory *factory)
-        : KitAspect(kit, factory), m_comboBox(createSubWidget<QComboBox>())
+        : KitAspect(kit, factory)
     {
         setManagingPage(Constants::Settings::TOOLS_ID);
-        m_comboBox->setSizePolicy(QSizePolicy::Ignored, m_comboBox->sizePolicy().verticalPolicy());
+
-        m_comboBox->setToolTip(factory->description());
         const auto sortModel = new CMakeToolSortModel(this);
         sortModel->setSourceModel(new CMakeToolListModel(*kit, this));
-        m_comboBox->setModel(sortModel);
+        auto getter = [](const Kit &k) { return CMakeKitAspect::cmakeToolId(&k).toSetting(); };
-
+        auto setter = [](Kit &k, const QVariant &id) {
-        refresh();
+            CMakeKitAspect::setCMakeTool(&k, Id::fromSetting(id));
-
+        };
-        connect(m_comboBox, &QComboBox::currentIndexChanged,
+        auto resetModel = [](QAbstractItemModel &model) {
-                this, &CMakeKitAspectImpl::currentCMakeToolChanged);
+            static_cast<CMakeToolSortModel &>(model).reset();
+        };
+        setListAspectSpec(
+            {sortModel,
+             std::move(getter),
+             std::move(setter),
+             std::move(resetModel),
+             CMakeToolTreeItem::IdRole});
 
         CMakeToolManager *cmakeMgr = CMakeToolManager::instance();
         connect(cmakeMgr, &CMakeToolManager::cmakeAdded, this, &CMakeKitAspectImpl::refresh);
         connect(cmakeMgr, &CMakeToolManager::cmakeRemoved, this, &CMakeKitAspectImpl::refresh);
         connect(cmakeMgr, &CMakeToolManager::cmakeUpdated, this, &CMakeKitAspectImpl::refresh);
     }
-
-    ~CMakeKitAspectImpl() override
-    {
-        delete m_comboBox;
-    }
-
-private:
-    // KitAspectWidget interface
-    void makeReadOnly() override { m_comboBox->setEnabled(false); }
-
-    void addToInnerLayout(Layouting::Layout &builder) override
-    {
-        addMutableAction(m_comboBox);
-        builder.addItem(m_comboBox);
-    }
-
-    void refresh() override
-    {
-        const GuardLocker locker(m_ignoreChanges);
-
-        const auto sortModel = static_cast<CMakeToolSortModel *>(m_comboBox->model());
-        sortModel->reset();
-        sortModel->sort(0);
-        m_comboBox->setCurrentIndex(indexOf(CMakeKitAspect::cmakeToolId(m_kit)));
-    }
-
-    int indexOf(Id id)
-    {
-        for (int i = 0; i < m_comboBox->count(); ++i) {
-            if (id == Id::fromSetting(m_comboBox->itemData(i, CMakeToolTreeItem::IdRole)))
-                return i;
-        }
-
-        return m_comboBox->count() - 1;
-    }
-
-    void currentCMakeToolChanged(int index)
-    {
-        if (m_ignoreChanges.isLocked())
-            return;
-
-        const Id id = Id::fromSetting(m_comboBox->itemData(index, CMakeToolTreeItem::IdRole));
-        CMakeKitAspect::setCMakeTool(m_kit, id);
-    }
-
-    Guard m_ignoreChanges;
-    QComboBox *m_comboBox;
 };
 
 CMakeKitAspectFactory::CMakeKitAspectFactory()

src/plugins/debugger/debuggerkitaspect.cpp

@@ -20,8 +20,6 @@
 #include <utils/processinterface.h>
 #include <utils/qtcassert.h>
 
-#include <QComboBox>
-
 #include <algorithm>
 
 using namespace ProjectExplorer;
@@ -117,59 +115,24 @@ public:
     {
         setManagingPage(ProjectExplorer::Constants::DEBUGGER_SETTINGS_PAGE_ID);
 
-        m_comboBox = createSubWidget<QComboBox>();
-        m_comboBox->setSizePolicy(QSizePolicy::Ignored, m_comboBox->sizePolicy().verticalPolicy());
-        m_comboBox->setEnabled(true);
         const auto sortModel = new DebuggerItemSortModel(this);
         sortModel->setSourceModel(new DebuggerItemListModel(*workingCopy, this));
-        m_comboBox->setModel(sortModel);
+        auto getter = [](const Kit &k) {
-
+            if (const DebuggerItem * const item = DebuggerKitAspect::debugger(&k))
-        refresh();
+                return item->id();
-        m_comboBox->setToolTip(factory->description());
+            return QVariant();
-        connect(m_comboBox, &QComboBox::currentIndexChanged, this, [this] {
+        };
-            if (m_ignoreChanges.isLocked())
+        auto setter = [](Kit &k, const QVariant &id) { k.setValue(DebuggerKitAspect::id(), id); };
-                return;
+        auto resetModel = [](QAbstractItemModel &model) {
-            m_kit->setValue(DebuggerKitAspect::id(), currentId());
+            static_cast<DebuggerItemSortModel &>(model).reset();
-        });
+        };
-
+        setListAspectSpec(
+            {sortModel,
+             std::move(getter),
+             std::move(setter),
+             std::move(resetModel),
+             DebuggerTreeItem::IdRole});
     }
-
-    ~DebuggerKitAspectImpl() override
-    {
-        delete m_comboBox;
-    }
-
-private:
-    void addToInnerLayout(Layouting::Layout &parent) override
-    {
-        addMutableAction(m_comboBox);
-        parent.addItem(m_comboBox);
-    }
-
-    void makeReadOnly() override
-    {
-        KitAspect::makeReadOnly();
-        m_comboBox->setEnabled(false);
-    }
-
-    void refresh() override
-    {
-        const GuardLocker locker(m_ignoreChanges);
-        const auto sortModel = static_cast<DebuggerItemSortModel *>(m_comboBox->model());
-        sortModel->reset();
-        sortModel->sort(0);
-        const DebuggerItem * const item = DebuggerKitAspect::debugger(m_kit);
-        m_comboBox->setCurrentIndex(
-            m_comboBox->findData(item ? item->id() : QVariant(), DebuggerTreeItem::IdRole));
-    }
-
-    QVariant currentId() const
-    {
-        return m_comboBox->itemData(m_comboBox->currentIndex(), DebuggerTreeItem::IdRole);
-    }
-
-    Guard m_ignoreChanges;
-    QComboBox *m_comboBox;
 };
 } // namespace Internal
 

src/plugins/projectexplorer/kitmanager.cpp

@@ -29,7 +29,9 @@
 
 #include <nanotrace/nanotrace.h>
 
+#include <QAbstractItemModel>
 #include <QAction>
+#include <QComboBox>
 #include <QHash>
 #include <QLabel>
 #include <QPushButton>
@@ -767,6 +769,17 @@ KitAspect::~KitAspect()
     delete m_mutableAction;
 }
 
+void KitAspect::refresh()
+{
+    if (!m_listAspectSpec)
+        return;
+    const GuardLocker locker(m_ignoreChanges);
+    m_listAspectSpec->resetModel(*m_listAspectSpec->model);
+    m_listAspectSpec->model->sort(0);
+    const QVariant itemId = m_listAspectSpec->getter(*kit());
+    m_comboBox->setCurrentIndex(m_comboBox->findData(itemId, m_listAspectSpec->itemRole));
+}
+
 void KitAspect::makeStickySubWidgetsReadOnly()
 {
     if (!m_kit->isSticky(m_factory->id()))
@@ -778,6 +791,38 @@ void KitAspect::makeStickySubWidgetsReadOnly()
     makeReadOnly();
 }
 
+void KitAspect::makeReadOnly()
+{
+    if (m_comboBox)
+        m_comboBox->setEnabled(false);
+}
+
+void KitAspect::addToInnerLayout(Layouting::Layout &parentItem)
+{
+    if (m_comboBox) {
+        addMutableAction(m_comboBox);
+        parentItem.addItem(m_comboBox);
+    }
+}
+
+void KitAspect::setListAspectSpec(ListAspectSpec &&listAspectSpec)
+{
+    m_listAspectSpec = std::move(listAspectSpec);
+
+    m_comboBox = createSubWidget<QComboBox>();
+    m_comboBox->setSizePolicy(QSizePolicy::Ignored, m_comboBox->sizePolicy().verticalPolicy());
+    m_comboBox->setEnabled(true);
+    m_comboBox->setModel(m_listAspectSpec->model);
+    m_comboBox->setToolTip(factory()->description()); // FIXME: We want the tooltip for the current item
+    refresh();
+    connect(m_comboBox, &QComboBox::currentIndexChanged, this, [this] {
+        if (m_ignoreChanges.isLocked())
+            return;
+        m_listAspectSpec->setter(
+            *kit(), m_comboBox->itemData(m_comboBox->currentIndex(), m_listAspectSpec->itemRole));
+    });
+}
+
 void KitAspect::addToLayoutImpl(Layouting::Layout &layout)
 {
     auto label = createSubWidget<QLabel>(m_factory->displayName() + ':');

src/plugins/projectexplorer/kitmanager.h

@@ -10,6 +10,7 @@
 #include <coreplugin/featureprovider.h>
 
 #include <utils/aspects.h>
+#include <utils/guard.h>
 
 #include <QPair>
 #include <QPushButton>
@@ -17,6 +18,11 @@
 
 #include <functional>
 
+QT_BEGIN_NAMESPACE
+class QAbstractItemModel;
+class QComboBox;
+QT_END_NAMESPACE
+
 namespace Utils {
 class Environment;
 class FilePath;
@@ -108,7 +114,7 @@ public:
     KitAspect(Kit *kit, const KitAspectFactory *factory);
     ~KitAspect();
 
-    virtual void refresh() = 0;
+    virtual void refresh();
 
     void addToLayoutImpl(Layouting::Layout &layout) override;
     static QString msgManage();
@@ -122,15 +128,48 @@ public:
     void makeStickySubWidgetsReadOnly();
 
 protected:
-    virtual void makeReadOnly() {}
+    virtual void makeReadOnly();
-    virtual void addToInnerLayout(Layouting::Layout &parentItem) = 0;
+    virtual void addToInnerLayout(Layouting::Layout &parentItem);
     virtual Utils::Id settingsPageItemToPreselect() const { return {}; }
 
+    // Convenience for aspects that provide a list model from which one value can be chosen.
+    // It will be exposed via a QComboBox.
+    class ListAspectSpec
+    {
+    public:
+        using Getter = std::function<QVariant(const Kit &)>;
+        using Setter = std::function<void(Kit &, const QVariant &)>;
+        using ResetModel = std::function<void(QAbstractItemModel &)>;
+
+        ListAspectSpec(
+            QAbstractItemModel *model,
+            Getter &&getter,
+            Setter &&setter,
+            ResetModel &&resetModel,
+            int itemRole)
+            : model(model)
+            , getter(std::move(getter))
+            , setter(std::move(setter))
+            , resetModel(std::move(resetModel))
+            , itemRole(itemRole)
+        {}
+
+        QAbstractItemModel *model;
+        Getter getter;
+        Setter setter;
+        ResetModel resetModel;
+        int itemRole;
+    };
+    void setListAspectSpec(ListAspectSpec &&listAspectSpec);
+
     Kit *m_kit;
     const KitAspectFactory *m_factory;
     QAction *m_mutableAction = nullptr;
     Utils::Id m_managingPageId;
     QPushButton *m_manageButton = nullptr;
+    QComboBox *m_comboBox = nullptr;
+    std::optional<ListAspectSpec> m_listAspectSpec;
+    Utils::Guard m_ignoreChanges;
 };
 
 class PROJECTEXPLORER_EXPORT KitManager final : public QObject

src/plugins/qtsupport/qtkitaspect.cpp

@@ -23,8 +23,6 @@
 #include <utils/macroexpander.h>
 #include <utils/qtcassert.h>
 
-#include <QComboBox>
-
 using namespace ProjectExplorer;
 using namespace Utils;
 
@@ -97,71 +95,27 @@ public:
     {
         setManagingPage(Constants::QTVERSION_SETTINGS_PAGE_ID);
 
-        m_combo = createSubWidget<QComboBox>();
-        m_combo->setSizePolicy(QSizePolicy::Ignored, m_combo->sizePolicy().verticalPolicy());
         const auto sortModel = new QtVersionSortModel(this);
         sortModel->setSourceModel(new QtVersionListModel(*k, this));
-        m_combo->setModel(sortModel);
+        auto getter = [](const Kit &k) { return QtKitAspect::qtVersionId(&k); };
-
+        auto setter = [](Kit &k, const QVariant &versionId) {
-        refresh();
+            QtKitAspect::setQtVersionId(&k, versionId.toInt());
-
+        };
-        // FIXME: We want the tooltip for the current item (also for toolchains etc).
+        auto resetModel = [](QAbstractItemModel &model) {
-        m_combo->setToolTip(ki->description());
+            static_cast<QtVersionSortModel &>(model).reset();
-
+        };
-        connect(m_combo, &QComboBox::currentIndexChanged, this, [this] {
+        setListAspectSpec(
-            if (!m_ignoreChanges.isLocked())
+            {sortModel,
-                currentWasChanged(m_combo->currentIndex());
+             std::move(getter),
-        });
+             std::move(setter),
+             std::move(resetModel),
+             QtVersionItem::IdRole});
 
         connect(KitManager::instance(), &KitManager::kitUpdated, this, [this](Kit *k) {
             if (k == kit())
                 refresh();
         });
     }
-
-    ~QtKitAspectImpl() final
-    {
-        delete m_combo;
-    }
-
-private:
-    void makeReadOnly() final { m_combo->setEnabled(false); }
-
-    void addToInnerLayout(Layouting::Layout &parent) override
-    {
-        addMutableAction(m_combo);
-        parent.addItem(m_combo);
-    }
-
-    void refresh() final
-    {
-        const GuardLocker locker(m_ignoreChanges);
-        const auto sortModel = static_cast<QtVersionSortModel *>(m_combo->model());
-        sortModel->reset();
-        sortModel->sort(0);
-        m_combo->setCurrentIndex(
-            m_combo->findData(QtKitAspect::qtVersionId(m_kit), QtVersionItem::IdRole));
-    }
-
-private:
-    static QString itemNameFor(const QtVersion *v)
-    {
-        QTC_ASSERT(v, return QString());
-        QString name = v->displayName();
-        if (!v->isValid())
-            name = Tr::tr("%1 (invalid)").arg(v->displayName());
-        return name;
-    }
-
-    void currentWasChanged(int idx)
-    {
-        const QAbstractItemModel * const model = m_combo->model();
-        const int versionId = model->data(model->index(idx, 0), QtVersionItem::IdRole).toInt();
-        QtKitAspect::setQtVersionId(m_kit, versionId);
-    }
-
-    Guard m_ignoreChanges;
-    QComboBox *m_combo;
 };
 } // namespace Internal