From 5f909ffe6200373b00267ba799ca3188abc60f89 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Wed, 2 Oct 2024 13:38:50 +0200 Subject: [PATCH] CMakeProjectManager: Use CMakeToolTreeItem in CMakeKitAspect For proper sorting and icons. Task-number: QTCREATORBUG-31574 Change-Id: I8c1a2df5251cc6d97a3c803d4d4c12c5f8847d71 Reviewed-by: hjk --- .../cmakeprojectmanager/cmakekitaspect.cpp | 107 ++++++-- .../cmakeprojectmanager/cmakesettingspage.cpp | 244 +++++++++--------- .../cmakeprojectmanager/cmakesettingspage.h | 47 +++- 3 files changed, 249 insertions(+), 149 deletions(-) diff --git a/src/plugins/cmakeprojectmanager/cmakekitaspect.cpp b/src/plugins/cmakeprojectmanager/cmakekitaspect.cpp index 22248e9d5a4..4461d3e133e 100644 --- a/src/plugins/cmakeprojectmanager/cmakekitaspect.cpp +++ b/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 +{ +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 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(sourceModel())->reset(); } + +private: + bool lessThan(const QModelIndex &source_left, const QModelIndex &source_right) const override + { + const auto source = static_cast(sourceModel()); + const auto item1 = static_cast(source->itemForIndex(source_left)); + const auto item2 = static_cast(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 FilePath rootPath = device->rootPath(); - - const QList toolsForBuildDevice - = 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(""), 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); + const auto sortModel = static_cast(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))) + 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; diff --git a/src/plugins/cmakeprojectmanager/cmakesettingspage.cpp b/src/plugins/cmakeprojectmanager/cmakesettingspage.cpp index d8ecf393751..a1cd3be7565 100644 --- a/src/plugins/cmakeprojectmanager/cmakesettingspage.cpp +++ b/src/plugins/cmakeprojectmanager/cmakesettingspage.cpp @@ -34,8 +34,6 @@ using namespace Utils; namespace CMakeProjectManager::Internal { -class CMakeToolTreeItem; - // // CMakeToolItemModel // @@ -68,139 +66,130 @@ public: QString uniqueDisplayName(const QString &base) const; private: + QVariant data(const QModelIndex &index, int role) const override; + Utils::Id m_defaultItemId; QList m_removedItems; }; -class CMakeToolTreeItem : public TreeItem +CMakeToolTreeItem::CMakeToolTreeItem(const CMakeTool *item, bool changed) + : m_id(item->id()) + , m_name(item->displayName()) + , m_executable(item->filePath()) + , m_qchFile(item->qchFilePath()) + , m_versionDisplay(item->versionDisplay()) + , m_detectionSource(item->detectionSource()) + , m_autodetected(item->isAutoDetected()) + , m_isSupported(item->hasFileApi()) + , m_changed(changed) { -public: - CMakeToolTreeItem(const CMakeTool *item, bool changed) - : m_id(item->id()) - , m_name(item->displayName()) - , m_executable(item->filePath()) - , m_qchFile(item->qchFilePath()) - , m_versionDisplay(item->versionDisplay()) - , m_detectionSource(item->detectionSource()) - , m_autodetected(item->isAutoDetected()) - , m_isSupported(item->hasFileApi()) - , m_changed(changed) - { - updateErrorFlags(); + updateErrorFlags(); +} + +CMakeToolTreeItem::CMakeToolTreeItem( + const QString &name, + const FilePath &executable, + const FilePath &qchFile, + bool autoRun, + bool autodetected) + : m_id(Id::generate()) + , m_name(name) + , m_executable(executable) + , m_qchFile(qchFile) + , m_isAutoRun(autoRun) + , m_autodetected(autodetected) +{ + 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::ManualDetection, m_id); + cmake.setFilePath(m_executable); + m_isSupported = cmake.hasFileApi(); + + m_tooltip = Tr::tr("Version: %1").arg(cmake.versionDisplay()); + m_tooltip += "
" + + Tr::tr("Supports fileApi: %1").arg(m_isSupported ? Tr::tr("yes") : Tr::tr("no")); + m_tooltip += "
" + Tr::tr("Detection source: \"%1\"").arg(m_detectionSource); + + m_versionDisplay = cmake.versionDisplay(); + + // Make sure to always have the right version in the name for Qt SDK CMake installations + if (m_autodetected && m_name.startsWith("CMake") && m_name.endsWith("(Qt)")) + m_name = QString("CMake %1 (Qt)").arg(m_versionDisplay); +} + +bool CMakeToolTreeItem::hasError() const +{ + 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 {}; } - CMakeToolTreeItem(const QString &name, - const FilePath &executable, - const FilePath &qchFile, - bool autoRun, - bool autodetected) - : m_id(Id::generate()) - , m_name(name) - , m_executable(executable) - , m_qchFile(qchFile) - , m_isAutoRun(autoRun) - , m_autodetected(autodetected) - { - updateErrorFlags(); - } - - void 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::ManualDetection, m_id); - cmake.setFilePath(m_executable); - m_isSupported = cmake.hasFileApi(); - - m_tooltip = Tr::tr("Version: %1").arg(cmake.versionDisplay()); - m_tooltip += "
" + Tr::tr("Supports fileApi: %1").arg(m_isSupported ? Tr::tr("yes") : Tr::tr("no")); - m_tooltip += "
" + Tr::tr("Detection source: \"%1\"").arg(m_detectionSource); - - m_versionDisplay = cmake.versionDisplay(); - - // Make sure to always have the right version in the name for Qt SDK CMake installations - if (m_autodetected && m_name.startsWith("CMake") && m_name.endsWith("(Qt)")) - m_name = QString("CMake %1 (Qt)").arg(m_versionDisplay); - } - - CMakeToolTreeItem() = default; - - CMakeToolItemModel *model() const { return static_cast(TreeItem::model()); } - - QVariant data(int column, int role) const override - { - switch (role) { - case Qt::DisplayRole: { - switch (column) { - case 0: { - QString name = m_name; - if (model()->defaultItemId() == m_id) - name += Tr::tr(" (Default)"); - return name; - } - case 1: { - return m_executable.toUserOutput(); - } - } // switch (column) - return QVariant(); - } - case Qt::FontRole: { - QFont font; - font.setBold(m_changed); - font.setItalic(model()->defaultItemId() == m_id); - return font; - } - case Qt::ToolTipRole: { - QString result = m_tooltip; - QString error; - if (!m_pathExists) { - error = Tr::tr("CMake executable path does not exist."); - } else if (!m_pathIsFile) { - error = Tr::tr("CMake executable path is not a file."); - } else if (!m_pathIsExecutable) { - error = Tr::tr("CMake executable path is not executable."); - } else if (!m_isSupported) { - error = Tr::tr( - "CMake executable does not provide required IDE integration features."); - } - if (result.isEmpty() || error.isEmpty()) - return QString("%1%2").arg(result).arg(error); - else - return QString("%1

%2").arg(result).arg(error); - } - case Qt::DecorationRole: { - if (column != 0) - return QVariant(); - - const bool hasError = !m_isSupported || !m_pathExists || !m_pathIsFile - || !m_pathIsExecutable; - if (hasError) - return Icons::CRITICAL.icon(); - return QVariant(); + switch (role) { + case Qt::DisplayRole: { + switch (column) { + case 0: { + QString name = m_name; + if (defaultItemId() == m_id) + name += Tr::tr(" (Default)"); + return name; } + case 1: { + return m_executable.toUserOutput(); } + } // switch (column) 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; -}; + case Qt::FontRole: { + QFont font; + font.setBold(m_changed); + font.setItalic(defaultItemId() == m_id); + return font; + } + case Qt::ToolTipRole: { + QString result = m_tooltip; + QString error; + if (!m_pathExists) { + error = Tr::tr("CMake executable path does not exist."); + } else if (!m_pathIsFile) { + error = Tr::tr("CMake executable path is not a file."); + } else if (!m_pathIsExecutable) { + error = Tr::tr("CMake executable path is not executable."); + } else if (!m_isSupported) { + error = Tr::tr("CMake executable does not provide required IDE integration features."); + } + if (result.isEmpty() || error.isEmpty()) + return QString("%1%2").arg(result).arg(error); + else + return QString("%1

%2").arg(result).arg(error); + } + case Qt::DecorationRole: { + if (column == 0 && hasError()) + return Icons::CRITICAL.icon(); + return QVariant(); + } + case IdRole: + return m_id.toSetting(); + } + return QVariant(); +} CMakeToolItemModel::CMakeToolItemModel() { @@ -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 // diff --git a/src/plugins/cmakeprojectmanager/cmakesettingspage.h b/src/plugins/cmakeprojectmanager/cmakesettingspage.h index 343bbe0438d..b48ffe3f080 100644 --- a/src/plugins/cmakeprojectmanager/cmakesettingspage.h +++ b/src/plugins/cmakeprojectmanager/cmakesettingspage.h @@ -3,8 +3,51 @@ #pragma once -namespace CMakeProjectManager::Internal { +#include +#include +#include + +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