ProjectExplorer: Use central KitAspectSortModel

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

src/plugins/cmakeprojectmanager/cmakekitaspect.cpp

@@ -98,34 +98,6 @@ private:
     const Kit &m_kit;
 };
 
-class CMakeToolSortModel : public SortModel
-{
-public:
-    CMakeToolSortModel(QObject *parent) : SortModel(parent) {}
-
-private:
-    bool lessThan(const QModelIndex &source_left, const QModelIndex &source_right) const override
-    {
-        const auto source = static_cast<CMakeToolListModel *>(sourceModel());
-        const auto item1 = static_cast<CMakeToolTreeItem *>(source->itemForIndex(source_left));
-        const auto item2 = static_cast<CMakeToolTreeItem *>(source->itemForIndex(source_right));
-        QTC_ASSERT(item1 && item2, return false);
-
-        // Criterion 1: "None" comes last
-        if (!item1->data(0, CMakeToolTreeItem::IdRole).isValid())
-            return false;
-        if (!item2->data(0, CMakeToolTreeItem::IdRole).isValid())
-            return true;
-
-        // Criterion 2: Tools with errors come after those without errors.
-        if (const bool item1Error = item1->hasError(); item1Error != item2->hasError())
-            return !item1Error;
-
-        // Criterion 3: Name.
-        return SortModel::lessThan(source_left, source_right);
-    }
-};
-
 // Factories
 
 class CMakeKitAspectFactory : public KitAspectFactory
@@ -190,19 +162,12 @@ public:
         setManagingPage(Constants::Settings::TOOLS_ID);
 
         const auto model = new CMakeToolListModel(*kit, this);
-        const auto sortModel = new CMakeToolSortModel(this);
-        sortModel->setSourceModel(model);
         auto getter = [](const Kit &k) { return CMakeKitAspect::cmakeToolId(&k).toSetting(); };
         auto setter = [](Kit &k, const QVariant &id) {
             CMakeKitAspect::setCMakeTool(&k, Id::fromSetting(id));
         };
         auto resetModel = [model] { model->reset(); };
-        setListAspectSpec(
+        setListAspectSpec({model, std::move(getter), std::move(setter), std::move(resetModel)});
-            {sortModel,
-             std::move(getter),
-             std::move(setter),
-             std::move(resetModel),
-             CMakeToolTreeItem::IdRole});
 
         CMakeToolManager *cmakeMgr = CMakeToolManager::instance();
         connect(cmakeMgr, &CMakeToolManager::cmakeAdded, this, &CMakeKitAspectImpl::refresh);

src/plugins/cmakeprojectmanager/cmakesettingspage.cpp

@@ -11,6 +11,7 @@
 #include <coreplugin/dialogs/ioptionspage.h>
 #include <coreplugin/icore.h>
 
+#include <projectexplorer/kitmanager.h>
 #include <projectexplorer/projectexplorerconstants.h>
 
 #include <utils/detailswidget.h>
@@ -139,6 +140,8 @@ QVariant CMakeToolTreeItem::data(int column, int role) const
     if (!m_id.isValid()) {
         if (role == Qt::DisplayRole && column == 0)
             return Tr::tr("None");
+        if (role == ProjectExplorer::KitAspect::IsNoneRole)
+            return true;
         return {};
     }
 
@@ -185,8 +188,10 @@ QVariant CMakeToolTreeItem::data(int column, int role) const
             return Icons::CRITICAL.icon();
         return QVariant();
     }
-    case IdRole:
+    case ProjectExplorer::KitAspect::IdRole:
         return m_id.toSetting();
+    case ProjectExplorer::KitAspect::QualityRole:
+        return int(!hasError());
     }
     return QVariant();
 }

src/plugins/cmakeprojectmanager/cmakesettingspage.h

@@ -29,7 +29,6 @@ public:
     void updateErrorFlags();
     bool hasError() const;
 
-    static const inline int IdRole = Qt::UserRole;
     static const int DefaultItemIdRole = Qt::UserRole + 1;
     QVariant data(int column, int role) const override;
 

src/plugins/debugger/debuggeritem.h

@@ -67,6 +67,7 @@ public:
     QStringList abiNames() const;
     QDateTime lastModified() const;
 
+    // Keep enum sorted ascending by goodness.
     enum class Problem { NoEngine, InvalidCommand, InvalidWorkingDir, None };
     Problem problem() const;
     QIcon decoration() const;

src/plugins/debugger/debuggeritemmanager.cpp

@@ -9,6 +9,7 @@
 #include <coreplugin/icore.h>
 
 #include <projectexplorer/devicesupport/devicemanager.h>
+#include <projectexplorer/kitmanager.h>
 #include <projectexplorer/kitoptionspage.h>
 #include <projectexplorer/projectexplorerconstants.h>
 #include <projectexplorer/projectexplorericons.h>
@@ -131,11 +132,14 @@ QVariant DebuggerTreeItem::data(int column, int role) const
     case Qt::ToolTipRole:
         return m_item.validityMessage();
 
-    case IdRole:
+    case KitAspect::IdRole:
         return m_item.id();
 
-    case ProblemRole:
+    case KitAspect::QualityRole:
         return int(m_item.problem());
+
+    case KitAspect::IsNoneRole:
+        return !m_item.isValid();
     }
     return QVariant();
 }

src/plugins/debugger/debuggeritemmanager.h

@@ -43,8 +43,6 @@ public:
         : m_item(item), m_orig(item), m_added(changed), m_changed(changed)
     {}
 
-    static const inline int IdRole = Qt::UserRole;
-    static const inline int ProblemRole = Qt::UserRole + 1;
     QVariant data(int column, int role) const override;
 
     DebuggerItem m_item; // Displayed, possibly unapplied data.

src/plugins/debugger/debuggerkitaspect.cpp

@@ -64,47 +64,6 @@ private:
     const Kit &m_kit;
 };
 
-class DebuggerItemSortModel : public SortModel
-{
-public:
-    DebuggerItemSortModel(QObject *parent) : SortModel(parent) {}
-
-private:
-    bool lessThan(const QModelIndex &source_left, const QModelIndex &source_right) const override
-    {
-        const auto source = static_cast<DebuggerItemListModel *>(sourceModel());
-        const auto item1 = static_cast<DebuggerTreeItem *>(source->itemForIndex(source_left));
-        const auto item2 = static_cast<DebuggerTreeItem *>(source->itemForIndex(source_right));
-        QTC_ASSERT(item1 && item2, return false);
-
-        // Criterion 1: "None" comes last
-        if (!item1->data(0, DebuggerTreeItem::IdRole).isValid())
-            return false;
-        if (!item2->data(0, DebuggerTreeItem::IdRole).isValid())
-            return true;
-
-        // Criterion 2: Invalid items come after valid ones with warnings, which come
-        //              after valid ones without warnings.
-        if (const QVariant &p1 = item1->data(0, DebuggerTreeItem::ProblemRole),
-                &p2 = item2->data(0, DebuggerTreeItem::ProblemRole);
-            p1 != p2) {
-            const auto problem1 = static_cast<DebuggerItem::Problem>(p1.toInt());
-            const auto problem2 = static_cast<DebuggerItem::Problem>(p2.toInt());
-            if (problem1 == DebuggerItem::Problem::None
-                || problem2 == DebuggerItem::Problem::NoEngine) {
-                return true;
-            }
-            if (problem2 == DebuggerItem::Problem::None
-                || problem1 == DebuggerItem::Problem::NoEngine) {
-                return false;
-            }
-        }
-
-        // Criterion 3: Name.
-        return SortModel::lessThan(source_left, source_right);
-    }
-};
-
 class DebuggerKitAspectImpl final : public KitAspect
 {
 public:
@@ -114,8 +73,6 @@ public:
         setManagingPage(ProjectExplorer::Constants::DEBUGGER_SETTINGS_PAGE_ID);
 
         const auto model = new DebuggerItemListModel(*workingCopy, this);
-        const auto sortModel = new DebuggerItemSortModel(this);
-        sortModel->setSourceModel(model);
         auto getter = [](const Kit &k) {
             if (const DebuggerItem * const item = DebuggerKitAspect::debugger(&k))
                 return item->id();
@@ -123,12 +80,7 @@ public:
         };
         auto setter = [](Kit &k, const QVariant &id) { k.setValue(DebuggerKitAspect::id(), id); };
         auto resetModel = [model] { model->reset(); };
-        setListAspectSpec(
+        setListAspectSpec({model, std::move(getter), std::move(setter), std::move(resetModel)});
-            {sortModel,
-             std::move(getter),
-             std::move(setter),
-             std::move(resetModel),
-             DebuggerTreeItem::IdRole});
     }
 };
 } // namespace Internal

src/plugins/projectexplorer/kitaspects.cpp

@@ -209,14 +209,12 @@ public:
                 return {};
             if (role == Qt::DisplayRole)
                 return std::get<0>(d);
-            if (role == Qt::UserRole)
+            if (role == KitAspect::IdRole)
                 return std::get<1>(d).toSetting();
             if (role == Qt::DecorationRole)
                 return std::get<2>(d);
             return {};
         });
-        const auto sortModel = new SortModel(this);
-        sortModel->setSourceModel(model);
         auto getter = [](const Kit &k) { return DeviceTypeKitAspect::deviceTypeId(&k).toSetting(); };
         auto setter = [](Kit &k, const QVariant &type) {
             DeviceTypeKitAspect::setDeviceTypeId(&k, Id::fromSetting(type));
@@ -229,7 +227,7 @@ public:
             }
         };
         setListAspectSpec(
-            {sortModel, std::move(getter), std::move(setter), std::move(resetModel), Qt::UserRole});
+            {model, std::move(getter), std::move(setter), std::move(resetModel)});
     }
 };
 } // namespace Internal

src/plugins/projectexplorer/kitmanager.cpp

@@ -26,6 +26,7 @@
 #include <utils/pointeralgorithm.h>
 #include <utils/qtcassert.h>
 #include <utils/stringutils.h>
+#include <utils/treemodel.h>
 
 #include <nanotrace/nanotrace.h>
 
@@ -68,6 +69,36 @@ static FilePath settingsFileName()
     return ICore::userResourcePath(KIT_FILENAME);
 }
 
+class KitAspectSortModel : public SortModel
+{
+public:
+    using SortModel::SortModel;
+
+private:
+    bool lessThan(const QModelIndex &source_left, const QModelIndex &source_right) const override
+    {
+        const auto getValue = [&](const QModelIndex &index, KitAspect::ItemRole role) {
+            return sourceModel()->data(index, role);
+        };
+
+        // Criterion 1: "None" comes last.
+        if (getValue(source_left, KitAspect::IsNoneRole).toBool())
+            return false;
+        if (getValue(source_right, KitAspect::IsNoneRole).toBool())
+            return true;
+
+        // Criterion 2: "Quality", i.e. how likely is the respective entry to be usable.
+        if (const int qual1 = getValue(source_left, KitAspect::QualityRole).toInt(),
+            qual2 = getValue(source_right, KitAspect::QualityRole).toInt();
+            qual1 != qual2) {
+            return qual1 > qual2;
+        }
+
+        // Criterion 3: Name.
+        return SortModel::lessThan(source_left, source_right);
+    }
+};
+
 // --------------------------------------------------------------------------
 // KitManagerPrivate:
 // --------------------------------------------------------------------------
@@ -775,9 +806,9 @@ void KitAspect::refresh()
         return;
     const GuardLocker locker(m_ignoreChanges);
     m_listAspectSpec->resetModel();
-    m_listAspectSpec->model->sort(0);
+    m_comboBox->model()->sort(0);
     const QVariant itemId = m_listAspectSpec->getter(*kit());
-    m_comboBox->setCurrentIndex(m_comboBox->findData(itemId, m_listAspectSpec->itemRole));
+    m_comboBox->setCurrentIndex(m_comboBox->findData(itemId, IdRole));
 }
 
 void KitAspect::makeStickySubWidgetsReadOnly()
@@ -812,7 +843,9 @@ void KitAspect::setListAspectSpec(ListAspectSpec &&listAspectSpec)
     m_comboBox = createSubWidget<QComboBox>();
     m_comboBox->setSizePolicy(QSizePolicy::Ignored, m_comboBox->sizePolicy().verticalPolicy());
     m_comboBox->setEnabled(true);
-    m_comboBox->setModel(m_listAspectSpec->model);
+    const auto sortModel = new KitAspectSortModel(this);
+    sortModel->setSourceModel(m_listAspectSpec->model);
+    m_comboBox->setModel(sortModel);
 
     refresh();
 
@@ -826,7 +859,7 @@ void KitAspect::setListAspectSpec(ListAspectSpec &&listAspectSpec)
             return;
         updateTooltip();
         m_listAspectSpec->setter(
-            *kit(), m_comboBox->itemData(m_comboBox->currentIndex(), m_listAspectSpec->itemRole));
+            *kit(), m_comboBox->itemData(m_comboBox->currentIndex(), IdRole));
     });
 }
 

src/plugins/projectexplorer/kitmanager.h

@@ -111,6 +111,8 @@ class PROJECTEXPLORER_EXPORT KitAspect : public Utils::BaseAspect
     Q_OBJECT
 
 public:
+    enum ItemRole { IdRole = Qt::UserRole + 100, IsNoneRole, QualityRole };
+
     KitAspect(Kit *kit, const KitAspectFactory *factory);
     ~KitAspect();
 
@@ -145,20 +147,17 @@ protected:
             QAbstractItemModel *model,
             Getter &&getter,
             Setter &&setter,
-            ResetModel &&resetModel,
+            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);
 

src/plugins/qtsupport/qtkitaspect.cpp

@@ -55,37 +55,6 @@ private:
     const Kit &m_kit;
 };
 
-class QtVersionSortModel : public SortModel
-{
-public:
-    QtVersionSortModel(QObject *parent) : SortModel(parent) {}
-
-private:
-    bool lessThan(const QModelIndex &source_left, const QModelIndex &source_right) const override
-    {
-        const auto source = static_cast<QtVersionListModel *>(sourceModel());
-        const auto item1 = static_cast<QtVersionItem *>(source->itemForIndex(source_left));
-        const auto item2 = static_cast<QtVersionItem *>(source->itemForIndex(source_right));
-        QTC_ASSERT(item1 && item2, return false);
-
-        // Criterion 1: "No Qt" comes last
-        if (item1->uniqueId() == -1)
-            return false;
-        if (item2->uniqueId() == -1)
-            return true;
-
-        // Criterion 2: Invalid Qt versions come after valid ones with warnings, which come
-        //              after valid ones without warnings.
-        if (const QtVersionItem::Quality qual1 = item1->quality(), qual2 = item2->quality();
-            qual1 != qual2) {
-            return qual1 == QtVersionItem::Quality::Good || qual2 == QtVersionItem::Quality::Bad;
-        }
-
-        // Criterion 3: Name.
-        return SortModel::lessThan(source_left, source_right);
-    }
-};
-
 class QtKitAspectImpl final : public KitAspect
 {
 public:
@@ -94,19 +63,12 @@ public:
         setManagingPage(Constants::QTVERSION_SETTINGS_PAGE_ID);
 
         const auto model = new QtVersionListModel(*k, this);
-        const auto sortModel = new QtVersionSortModel(this);
-        sortModel->setSourceModel(model);
         auto getter = [](const Kit &k) { return QtKitAspect::qtVersionId(&k); };
         auto setter = [](Kit &k, const QVariant &versionId) {
             QtKitAspect::setQtVersionId(&k, versionId.toInt());
         };
         auto resetModel = [model] { model->reset(); };
-        setListAspectSpec(
+        setListAspectSpec({model, std::move(getter), std::move(setter), std::move(resetModel)});
-            {sortModel,
-             std::move(getter),
-             std::move(setter),
-             std::move(resetModel),
-             QtVersionItem::IdRole});
 
         connect(KitManager::instance(), &KitManager::kitUpdated, this, [this](Kit *k) {
             if (k == kit())

src/plugins/qtsupport/qtoptionspage.cpp

@@ -13,6 +13,7 @@
 #include <coreplugin/icore.h>
 #include <coreplugin/progressmanager/progressmanager.h>
 
+#include <projectexplorer/kitmanager.h>
 #include <projectexplorer/kitoptionspage.h>
 #include <projectexplorer/projectexplorerconstants.h>
 #include <projectexplorer/projectexplorericons.h>
@@ -165,9 +166,11 @@ QVariant QtVersionItem::data(int column, int role) const
     const QtVersion *version = this->version();
 
     if (!version) {
+        if (role == KitAspect::IsNoneRole && column == 0)
+            return true;
         if (role == Qt::DisplayRole && column == 0)
             return Tr::tr("None");
-        if (role == IdRole)
+        if (role == KitAspect::IdRole)
             return -1;
         return TreeItem::data(column, role);
     }
@@ -221,9 +224,12 @@ QVariant QtVersionItem::data(int column, int role) const
         return desc;
     }
 
-    if (role == IdRole)
+    if (role == KitAspect::IdRole)
         return uniqueId();
 
+    if (role == KitAspect::QualityRole)
+        return int(quality());
+
     return QVariant();
 }
 

src/plugins/qtsupport/qtoptionspage.h

@@ -35,10 +35,9 @@ public:
     void setChanged(bool changed);
     void setIsNameUnique(const std::function<bool(QtVersion *)> &isNameUnique);
 
-    enum class Quality { Good, Limited, Bad };
+    enum class Quality { Bad, Limited, Good }; // Keep sorted ascending by goodness.
     Quality quality() const;
 
-    static const int IdRole = Qt::UserRole;
 private:
     QVariant data(int column, int role) const final;