CMakeProjectManager: Use CMakeToolTreeItem in CMakeKitAspect

For proper sorting and icons.

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

src/plugins/cmakeprojectmanager/cmakekitaspect.cpp

@@ -6,6 +6,7 @@
 #include "cmakeconfigitem.h"
 #include "cmakeprojectconstants.h"
 #include "cmakeprojectmanagertr.h"
+#include "cmakesettingspage.h"
 #include "cmakespecificsettings.h"
 #include "cmaketool.h"
 #include "cmaketoolmanager.h"
@@ -49,6 +50,7 @@ using namespace ProjectExplorer;
 using namespace Utils;
 
 namespace CMakeProjectManager {
+namespace Internal {
 
 static bool isIos(const Kit *k)
 {
@@ -63,6 +65,71 @@ static Id defaultCMakeToolId()
     return defaultTool ? defaultTool->id() : Id();
 }
 
+class CMakeToolListModel : public TreeModel<TreeItem, Internal::CMakeToolTreeItem>
+{
+public:
+    CMakeToolListModel(const Kit &kit, QObject *parent)
+        : TreeModel(parent)
+        , m_kit(kit)
+    {}
+
+    void reset()
+    {
+        clear();
+
+        const FilePath rootPath = BuildDeviceKitAspect::device(&m_kit)->rootPath();
+        const QList<CMakeTool *> toolsForBuildDevice
+            = Utils::filtered(CMakeToolManager::cmakeTools(), [rootPath](CMakeTool *item) {
+                  return item->cmakeExecutable().isSameDevice(rootPath);
+              });
+        for (CMakeTool *item : toolsForBuildDevice)
+            rootItem()->appendChild(new CMakeToolTreeItem(item, false));
+
+        // TODO: The aspect actively prevents the "no value" case in several places
+        // rootItem()->appendChild(new CMakeToolTreeItem);
+    }
+
+private:
+    QVariant data(const QModelIndex &index, int role) const
+    {
+        if (role == CMakeToolTreeItem::DefaultItemIdRole)
+            return defaultCMakeToolId().toSetting();
+        return TreeModel::data(index, role);
+    }
+
+    const Kit &m_kit;
+};
+
+class CMakeToolSortModel : public SortModel
+{
+public:
+    CMakeToolSortModel(QObject *parent) : SortModel(parent) {}
+
+    void reset() { static_cast<CMakeToolListModel *>(sourceModel())->reset(); }
+
+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
@@ -126,8 +193,10 @@ public:
     {
         setManagingPage(Constants::Settings::TOOLS_ID);
         m_comboBox->setSizePolicy(QSizePolicy::Ignored, m_comboBox->sizePolicy().verticalPolicy());
-        m_comboBox->setEnabled(false);
         m_comboBox->setToolTip(factory->description());
+        const auto sortModel = new CMakeToolSortModel(this);
+        sortModel->setSourceModel(new CMakeToolListModel(*kit, this));
+        m_comboBox->setModel(sortModel);
 
         refresh();
 
@@ -158,35 +227,23 @@ private:
     void refresh() override
     {
         const GuardLocker locker(m_ignoreChanges);
-        m_comboBox->clear();
 
-        IDeviceConstPtr device = BuildDeviceKitAspect::device(kit());
+        const auto sortModel = static_cast<CMakeToolSortModel *>(m_comboBox->model());
-        const FilePath rootPath = device->rootPath();
+        sortModel->reset();
-
+        sortModel->sort(0);
-        const QList<CMakeTool *> toolsForBuildDevice
+        m_comboBox->setCurrentIndex(indexOf(CMakeKitAspect::cmakeToolId(m_kit)));
-            = Utils::filtered(CMakeToolManager::cmakeTools(), [rootPath](CMakeTool *item) {
-                  return item->cmakeExecutable().isSameDevice(rootPath);
-              });
-
-        m_comboBox->setEnabled(!toolsForBuildDevice.isEmpty());
-        if (toolsForBuildDevice.isEmpty()) {
-            m_comboBox->addItem(Tr::tr("<No CMake Tool available>"), Id().toSetting());
-            return;
-        }
-
-        for (CMakeTool *item : toolsForBuildDevice)
-            m_comboBox->addItem(item->displayName(), item->id().toSetting());
-
-        CMakeTool *tool = CMakeKitAspect::cmakeTool(m_kit);
-        m_comboBox->setCurrentIndex(tool ? indexOf(tool->id()) : -1);
     }
 
     int indexOf(Id id)
     {
         for (int i = 0; i < m_comboBox->count(); ++i) {
-            if (id == Id::fromSetting(m_comboBox->itemData(i)))
+            if (id == Id::fromSetting(m_comboBox->itemData(i, CMakeToolTreeItem::IdRole)))
                 return i;
         }
+
+        // TODO: Enable once we have "none" entry.
+        // return m_comboBox->count() - 1;
+
         return -1;
     }
 
@@ -195,7 +252,7 @@ private:
         if (m_ignoreChanges.isLocked())
             return;
 
-        const Id id = Id::fromSetting(m_comboBox->itemData(index));
+        const Id id = Id::fromSetting(m_comboBox->itemData(index, CMakeToolTreeItem::IdRole));
         CMakeKitAspect::setCMakeTool(m_kit, id);
     }
 
@@ -226,6 +283,10 @@ CMakeKitAspectFactory::CMakeKitAspectFactory()
             this, updateKits);
 }
 
+} // Internal
+
+using namespace Internal;
+
 Id CMakeKitAspect::id()
 {
     return Constants::TOOL_ID;

src/plugins/cmakeprojectmanager/cmakesettingspage.cpp

@@ -34,8 +34,6 @@ using namespace Utils;
 
 namespace CMakeProjectManager::Internal {
 
-class CMakeToolTreeItem;
-
 //
 // CMakeToolItemModel
 //
@@ -68,14 +66,13 @@ public:
 
     QString uniqueDisplayName(const QString &base) const;
 private:
+    QVariant data(const QModelIndex &index, int role) const override;
+
     Utils::Id m_defaultItemId;
     QList<Utils::Id> m_removedItems;
 };
 
-class CMakeToolTreeItem : public TreeItem
+CMakeToolTreeItem::CMakeToolTreeItem(const CMakeTool *item, bool changed)
-{
-public:
-    CMakeToolTreeItem(const CMakeTool *item, bool changed)
     : m_id(item->id())
     , m_name(item->displayName())
     , m_executable(item->filePath())
@@ -89,7 +86,8 @@ public:
     updateErrorFlags();
 }
 
-    CMakeToolTreeItem(const QString &name,
+CMakeToolTreeItem::CMakeToolTreeItem(
+    const QString &name,
     const FilePath &executable,
     const FilePath &qchFile,
     bool autoRun,
@@ -104,20 +102,20 @@ public:
     updateErrorFlags();
 }
 
-    void updateErrorFlags()
+void CMakeToolTreeItem::updateErrorFlags()
 {
     const FilePath filePath = CMakeTool::cmakeExecutable(m_executable);
     m_pathExists = filePath.exists();
     m_pathIsFile = filePath.isFile();
     m_pathIsExecutable = filePath.isExecutableFile();
 
-        CMakeTool cmake(m_autodetected ? CMakeTool::AutoDetection
+    CMakeTool cmake(m_autodetected ? CMakeTool::AutoDetection : CMakeTool::ManualDetection, m_id);
-                                       : CMakeTool::ManualDetection, m_id);
     cmake.setFilePath(m_executable);
     m_isSupported = cmake.hasFileApi();
 
     m_tooltip = Tr::tr("Version: %1").arg(cmake.versionDisplay());
-        m_tooltip += "<br>" + Tr::tr("Supports fileApi: %1").arg(m_isSupported ? Tr::tr("yes") : Tr::tr("no"));
+    m_tooltip += "<br>"
+                 + Tr::tr("Supports fileApi: %1").arg(m_isSupported ? Tr::tr("yes") : Tr::tr("no"));
     m_tooltip += "<br>" + Tr::tr("Detection source: \"%1\"").arg(m_detectionSource);
 
     m_versionDisplay = cmake.versionDisplay();
@@ -127,18 +125,29 @@ public:
         m_name = QString("CMake %1 (Qt)").arg(m_versionDisplay);
 }
 
-    CMakeToolTreeItem() = default;
+bool CMakeToolTreeItem::hasError() const
-
-    CMakeToolItemModel *model() const { return static_cast<CMakeToolItemModel *>(TreeItem::model()); }
-
-    QVariant data(int column, int role) const override
 {
+    return !m_isSupported || !m_pathExists || !m_pathIsFile || !m_pathIsExecutable;
+}
+
+QVariant CMakeToolTreeItem::data(int column, int role) const
+{
+    const auto defaultItemId = [this] {
+        return Id::fromSetting(model()->data({}, DefaultItemIdRole));
+    };
+
+    if (!m_id.isValid()) {
+        if (role == Qt::DisplayRole && column == 0)
+            return Tr::tr("None");
+        return {};
+    }
+
     switch (role) {
     case Qt::DisplayRole: {
         switch (column) {
         case 0: {
             QString name = m_name;
-                if (model()->defaultItemId() == m_id)
+            if (defaultItemId() == m_id)
                 name += Tr::tr(" (Default)");
             return name;
         }
@@ -151,7 +160,7 @@ public:
     case Qt::FontRole: {
         QFont font;
         font.setBold(m_changed);
-            font.setItalic(model()->defaultItemId() == m_id);
+        font.setItalic(defaultItemId() == m_id);
         return font;
     }
     case Qt::ToolTipRole: {
@@ -164,8 +173,7 @@ public:
         } else if (!m_pathIsExecutable) {
             error = Tr::tr("CMake executable path is not executable.");
         } else if (!m_isSupported) {
-                error = Tr::tr(
+            error = Tr::tr("CMake executable does not provide required IDE integration features.");
-                    "CMake executable does not provide required IDE integration features.");
         }
         if (result.isEmpty() || error.isEmpty())
             return QString("%1%2").arg(result).arg(error);
@@ -173,35 +181,16 @@ public:
             return QString("%1<br><br><b>%2</b>").arg(result).arg(error);
     }
     case Qt::DecorationRole: {
-            if (column != 0)
+        if (column == 0 && hasError())
-                return QVariant();
-
-            const bool hasError = !m_isSupported || !m_pathExists || !m_pathIsFile
-                                  || !m_pathIsExecutable;
-            if (hasError)
             return Icons::CRITICAL.icon();
         return QVariant();
     }
+    case IdRole:
+        return m_id.toSetting();
     }
     return QVariant();
 }
 
-    Id m_id;
-    QString m_name;
-    QString m_tooltip;
-    FilePath m_executable;
-    FilePath m_qchFile;
-    QString m_versionDisplay;
-    QString m_detectionSource;
-    bool m_isAutoRun = true;
-    bool m_pathExists = false;
-    bool m_pathIsFile = false;
-    bool m_pathIsExecutable = false;
-    bool m_autodetected = false;
-    bool m_isSupported = false;
-    bool m_changed = true;
-};
-
 CMakeToolItemModel::CMakeToolItemModel()
 {
     setHeader({Tr::tr("Name"), Tr::tr("Path")});
@@ -382,6 +371,13 @@ QString CMakeToolItemModel::uniqueDisplayName(const QString &base) const
     return Utils::makeUniquelyNumbered(base, names);
 }
 
+QVariant CMakeToolItemModel::data(const QModelIndex &index, int role) const
+{
+    if (role == CMakeToolTreeItem::DefaultItemIdRole)
+        return defaultItemId().toSetting();
+    return TreeModel::data(index, role);
+}
+
 //
 // CMakeToolItemConfigWidget
 //

src/plugins/cmakeprojectmanager/cmakesettingspage.h

@@ -3,8 +3,51 @@
 
 #pragma once
 
-namespace CMakeProjectManager::Internal {
+#include <utils/filepath.h>
+#include <utils/id.h>
+#include <utils/treemodel.h>
+
+namespace CMakeProjectManager {
+class CMakeTool;
+
+namespace Internal {
 
 void setupCMakeSettingsPage();
 
-} // CMakeProjectManager::Internal
+class CMakeToolTreeItem : public Utils::TreeItem
+{
+public:
+    CMakeToolTreeItem() = default;
+    CMakeToolTreeItem(const CMakeTool *item, bool changed);
+    CMakeToolTreeItem(
+        const QString &name,
+        const Utils::FilePath &executable,
+        const Utils::FilePath &qchFile,
+        bool autoRun,
+        bool autodetected);
+
+    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;
+
+    Utils::Id m_id;
+    QString m_name;
+    QString m_tooltip;
+    Utils::FilePath m_executable;
+    Utils::FilePath m_qchFile;
+    QString m_versionDisplay;
+    QString m_detectionSource;
+    bool m_isAutoRun = true;
+    bool m_pathExists = false;
+    bool m_pathIsFile = false;
+    bool m_pathIsExecutable = false;
+    bool m_autodetected = false;
+    bool m_isSupported = false;
+    bool m_changed = true;
+};
+
+} // Internal
+} // CMakeProjectManager