QtSupport: Use QtVersionItem in QtKitAspect

For proper sorting and icons.

Task-number: QTCREATORBUG-31574
Change-Id: Ib835091684527ce22b0294c494227979e3b093ec
Reviewed-by: hjk <hjk@qt.io>

src/plugins/qtsupport/qtkitaspect.cpp

@@ -3,6 +3,7 @@
 
 #include "qtkitaspect.h"
 
+#include "qtoptionspage.h"
 #include "qtparser.h"
 #include "qtsupportconstants.h"
 #include "qtsupporttr.h"
@@ -30,6 +31,80 @@ using namespace Utils;
 namespace QtSupport {
 namespace Internal {
 
+class QtVersionListModel : public TreeModel<TreeItem, QtVersionItem>
+{
+public:
+    QtVersionListModel(const Kit &kit, QObject *parent)
+        : TreeModel(parent)
+        , m_kit(kit)
+    {}
+
+    QModelIndex indexForQtId(int id) const
+    {
+        if (id == -1)
+            return index(rowCount() - 1, 0); // The "No Qt" item always comes last
+        const TreeItem *const item = findItemAtLevel<1>(
+            [id](TreeItem *item) { return static_cast<QtVersionItem *>(item)->uniqueId() == id; });
+        return item ? indexForItem(item) : QModelIndex();
+    }
+
+    void reset()
+    {
+        clear();
+
+        const FilePath deviceRoot = BuildDeviceKitAspect::device(&m_kit)->rootPath();
+        const QtVersions versionsForBuildDevice = QtVersionManager::versions(
+            [&deviceRoot](const QtVersion *qt) {
+                return qt->qmakeFilePath().isSameDevice(deviceRoot);
+            });
+        for (QtVersion *v : versionsForBuildDevice)
+            rootItem()->appendChild(new QtVersionItem(v->uniqueId()));
+        rootItem()->appendChild(new QtVersionItem(-1)); // The "No Qt" entry.
+    }
+
+private:
+    const Kit &m_kit;
+};
+
+class QtVersionSortModel : public SortModel
+{
+public:
+    QtVersionSortModel(QObject *parent) : SortModel(parent) {}
+
+    QModelIndex indexForId(int id) const
+    {
+        return mapFromSource(
+            static_cast<QtVersionListModel *>(sourceModel())->indexForQtId(id));
+    }
+
+    void reset() { static_cast<QtVersionListModel *>(sourceModel())->reset(); }
+
+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:
@@ -39,8 +114,13 @@ public:
 
         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);
 
         refresh();
+
+        // FIXME: We want the tooltip for the current item (also for toolchains etc).
         m_combo->setToolTip(ki->description());
 
         connect(m_combo, &QComboBox::currentIndexChanged, this, [this] {
@@ -48,10 +128,10 @@ public:
                 currentWasChanged(m_combo->currentIndex());
         });
 
-        connect(QtVersionManager::instance(),
+        connect(KitManager::instance(), &KitManager::kitUpdated, this, [this](Kit *k) {
-                &QtVersionManager::qtVersionsChanged,
+            if (k == kit())
-                this,
+                refresh();
-                &QtKitAspectImpl::refresh);
+        });
     }
 
     ~QtKitAspectImpl() final
@@ -71,20 +151,10 @@ private:
     void refresh() final
     {
         const GuardLocker locker(m_ignoreChanges);
-        m_combo->clear();
+        const auto sortModel = static_cast<QtVersionSortModel *>(m_combo->model());
-        m_combo->addItem(Tr::tr("None"), -1);
+        sortModel->reset();
-
+        sortModel->sort(0);
-        IDeviceConstPtr device = BuildDeviceKitAspect::device(kit());
+        m_combo->setCurrentIndex(sortModel->indexForId(QtKitAspect::qtVersionId(m_kit)).row());
-        const FilePath deviceRoot = device->rootPath();
-
-        const QtVersions versionsForBuildDevice
-            = Utils::filtered(QtVersionManager::versions(), [device](QtVersion *qt) {
-                  return qt->qmakeFilePath().isSameDevice(device->rootPath());
-              });
-
-        for (QtVersion *item : versionsForBuildDevice)
-            m_combo->addItem(item->displayName(), item->uniqueId());
-        m_combo->setCurrentIndex(findQtVersion(QtKitAspect::qtVersionId(m_kit)));
     }
 
 private:
@@ -99,16 +169,9 @@ private:
 
     void currentWasChanged(int idx)
     {
-        QtKitAspect::setQtVersionId(m_kit, m_combo->itemData(idx).toInt());
+        const QAbstractItemModel * const model = m_combo->model();
-    }
+        const int versionId = model->data(model->index(idx, 0), QtVersionItem::IdRole).toInt();
-
+        QtKitAspect::setQtVersionId(m_kit, versionId);
-    int findQtVersion(const int id) const
-    {
-        for (int i = 0; i < m_combo->count(); ++i) {
-            if (id == m_combo->itemData(i).toInt())
-                return i;
-        }
-        return -1;
     }
 
     Guard m_ignoreChanges;

src/plugins/qtsupport/qtoptionspage.cpp

@@ -27,7 +27,6 @@
 #include <utils/layoutbuilder.h>
 #include <utils/pathchooser.h>
 #include <utils/qtcassert.h>
-#include <utils/treemodel.h>
 #include <utils/utilsicons.h>
 #include <utils/variablechooser.h>
 
@@ -44,7 +43,6 @@
 #include <QTextBrowser>
 #include <QTreeView>
 
-#include <functional>
 #include <utility>
 
 using namespace Core;
@@ -128,44 +126,57 @@ UnsupportedAbisInfo checkForUnsupportedAbis(const QtVersion *version)
     return info;
 }
 
-class QtVersionItem : public TreeItem
+void QtVersionItem::setIsNameUnique(const std::function<bool(QtVersion *)> &isNameUnique)
 {
-public:
+    m_isNameUnique = isNameUnique;
-    explicit QtVersionItem(QtVersion *version)
-        : m_version(version)
-    {}
-
-    ~QtVersionItem()
-    {
-        delete m_version;
 }
 
-    void setVersion(QtVersion *version)
+QtVersionItem::Quality QtVersionItem::quality() const
 {
-        m_version = version;
+    const QtVersion *version = this->version();
+    QTC_ASSERT(version, return Quality::Bad);
+
+    if (!version->isValid())
+        return Quality::Bad;
+    if (!version->warningReason().isEmpty() || hasNonUniqueDisplayName())
+        return Quality::Limited;
+    const UnsupportedAbisInfo abisInfo = checkForUnsupportedAbis(version);
+    switch (abisInfo.status) {
+    case UnsupportedAbisInfo::Status::AllMissing:
+        return Quality::Bad;
+    case UnsupportedAbisInfo::Status::SomeMissing:
+        return Quality::Limited;
+    case UnsupportedAbisInfo::Status::Ok:
+        break;
+    }
+    return Quality::Good;
+}
+
+void QtVersionItem::setChanged(bool changed)
+{
+    if (changed == m_changed)
+        return;
+    m_changed = changed;
     update();
 }
 
-    int uniqueId() const
+QVariant QtVersionItem::data(int column, int role) const
 {
-        return m_version ? m_version->uniqueId() : -1;
+    const QtVersion *version = this->version();
-    }
 
-    QtVersion *version() const
+    if (!version) {
-    {
+        if (role == Qt::DisplayRole && column == 0)
-        return m_version;
+            return Tr::tr("No Qt");
-    }
+        if (role == IdRole)
-
+            return -1;
-    QVariant data(int column, int role) const final
-    {
-        if (!m_version)
         return TreeItem::data(column, role);
+    }
 
     if (role == Qt::DisplayRole) {
         if (column == 0)
-                return m_version->displayName();
+            return version->displayName();
         if (column == 1)
-                return m_version->qmakeFilePath().toUserOutput();
+            return version->qmakeFilePath().toUserOutput();
     }
 
     if (role == Qt::FontRole && m_changed) {
@@ -175,40 +186,34 @@ public:
     }
 
     if (role == Qt::DecorationRole && column == 0) {
-             if (!m_version->isValid())
+        switch (quality()) {
-                return invalidVersionIcon();
+        case Quality::Good:
-             if (!m_version->warningReason().isEmpty() || hasNonUniqueDisplayName())
-                 return warningVersionIcon();
-             const UnsupportedAbisInfo abisInfo = checkForUnsupportedAbis(m_version);
-             switch (abisInfo.status) {
-             case UnsupportedAbisInfo::Status::AllMissing:
-                 return invalidVersionIcon();
-             case UnsupportedAbisInfo::Status::SomeMissing:
-                 return warningVersionIcon();
-             case UnsupportedAbisInfo::Status::Ok:
-                 break;
-             }
             return validVersionIcon();
+        case Quality::Limited:
+            return warningVersionIcon();
+        case Quality::Bad:
+            return invalidVersionIcon();
+        }
     }
 
     if (role == Qt::ToolTipRole) {
         const QString row = "<dt style=\"font-weight:bold\">%1:</dt>"
                             "<dd>%2</dd>";
         QString desc = "<dl style=\"white-space:pre\">";
-            if (m_version->isValid())
+        if (version->isValid())
-                desc += row.arg(Tr::tr("Qt Version"), m_version->qtVersionString());
+            desc += row.arg(Tr::tr("Qt Version"), version->qtVersionString());
-            desc += row.arg(Tr::tr("Location of qmake"), m_version->qmakeFilePath().toUserOutput());
+        desc += row.arg(Tr::tr("Location of qmake"), version->qmakeFilePath().toUserOutput());
-            if (m_version->isValid()) {
+        if (version->isValid()) {
-                const UnsupportedAbisInfo abisInfo = checkForUnsupportedAbis(m_version);
+            const UnsupportedAbisInfo abisInfo = checkForUnsupportedAbis(version);
             if (abisInfo.status == UnsupportedAbisInfo::Status::AllMissing)
                 desc += row.arg(Tr::tr("Error"), abisInfo.message);
             if (abisInfo.status == UnsupportedAbisInfo::Status::SomeMissing)
                 desc += row.arg(Tr::tr("Warning"), abisInfo.message);
-                const QStringList warnings = m_version->warningReason();
+            const QStringList warnings = version->warningReason();
             for (const QString &w : warnings)
                 desc += row.arg(Tr::tr("Warning"), w);
         } else {
-                desc += row.arg(Tr::tr("Error"), m_version->invalidReason());
+            desc += row.arg(Tr::tr("Error"), version->invalidReason());
         }
         if (hasNonUniqueDisplayName())
             desc += row.arg(Tr::tr("Warning"), nonUniqueDisplayNameWarning());
@@ -216,30 +221,38 @@ public:
         return desc;
     }
 
+    if (role == IdRole)
+        return uniqueId();
+
     return QVariant();
 }
 
-    void setChanged(bool changed)
+int QtVersionItem::uniqueId() const
 {
-        if (changed == m_changed)
+    if (const auto v = std::get_if<QtVersion *>(&m_version))
-            return;
+        return v ? (*v)->uniqueId() : -1;
-        m_changed = changed;
+    return *std::get_if<int>(&m_version);
+}
+
+QtVersion *QtVersionItem::version() const
+{
+    if (const auto v = std::get_if<QtVersion *>(&m_version))
+        return *v;
+    return QtVersionManager::version(*std::get_if<int>(&m_version));
+}
+
+void QtVersionItem::setVersion(QtVersion *version)
+{
+    m_version = version;
     update();
 }
 
-    void setIsNameUnique(const std::function<bool(QtVersion *)> &isNameUnique)
+QtVersionItem::~QtVersionItem()
 {
-        m_isNameUnique = isNameUnique;
+    if (const auto v = std::get_if<QtVersion *>(&m_version))
+        delete *v;
 }
 
-private:
-    bool hasNonUniqueDisplayName() const { return m_isNameUnique && !m_isNameUnique(m_version); }
-
-    QtVersion *m_version = nullptr;
-    std::function<bool(QtVersion *)> m_isNameUnique;
-    bool m_changed = false;
-};
-
 // QtSettingsPageWidget
 
 class QtSettingsPageWidget final : public IOptionsPageWidget
@@ -1130,7 +1143,7 @@ void setupQtSettingsPage()
     static QtSettingsPage theQtSettingsPage;
 }
 
-} // Internal
+} // namespace Internal
 
 bool LinkWithQtSupport::canLinkWithQt()
 {

src/plugins/qtsupport/qtoptionspage.h

@@ -6,8 +6,13 @@
 #include "qtsupport_global.h"
 
 #include <coreplugin/dialogs/ioptionspage.h>
+#include <utils/treemodel.h>
+
+#include <functional>
+#include <variant>
 
 namespace QtSupport {
+class QtVersion;
 
 namespace LinkWithQtSupport {
 QTSUPPORT_EXPORT bool canLinkWithQt();
@@ -16,6 +21,35 @@ QTSUPPORT_EXPORT Utils::FilePath linkedQt();
 QTSUPPORT_EXPORT void linkWithQt();
 }
 
-namespace Internal { void setupQtSettingsPage(); }
+namespace Internal {
+class QtVersionItem : public Utils::TreeItem
+{
+public:
+    explicit QtVersionItem(QtVersion *version) : m_version(version) {}
+    explicit QtVersionItem(int versionId) : m_version(versionId) {}
+    ~QtVersionItem();
+
+    void setVersion(QtVersion *version);
+    int uniqueId() const;
+    QtVersion *version() const;
+    void setChanged(bool changed);
+    void setIsNameUnique(const std::function<bool(QtVersion *)> &isNameUnique);
+
+    enum class Quality { Good, Limited, Bad };
+    Quality quality() const;
+
+    static const int IdRole = Qt::UserRole;
+private:
+    QVariant data(int column, int role) const final;
+
+    bool hasNonUniqueDisplayName() const { return m_isNameUnique && !m_isNameUnique(version()); }
+
+    std::variant<QtVersion *, int> m_version;
+    std::function<bool(QtVersion *)> m_isNameUnique;
+    bool m_changed = false;
+};
+
+void setupQtSettingsPage();
+}
 
 } // QtSupport