From 41df91ece8e2d88fabbaf00b2074b810ba93b51a Mon Sep 17 00:00:00 2001 From: Tobias Hunger Date: Thu, 8 Mar 2018 13:01:18 +0100 Subject: [PATCH] TargetSetupWidget: Replace a range of lists with a list of struct Simpler to make sure all the data is in place at all times that way. Change-Id: I73d88f4c31d8447547ccf6de808ea00066db4f37 Reviewed-by: hjk --- .../projectexplorer/targetsetupwidget.cpp | 154 +++++++++--------- .../projectexplorer/targetsetupwidget.h | 28 +++- 2 files changed, 99 insertions(+), 83 deletions(-) diff --git a/src/plugins/projectexplorer/targetsetupwidget.cpp b/src/plugins/projectexplorer/targetsetupwidget.cpp index c5aaab3c881..f5213a9071a 100644 --- a/src/plugins/projectexplorer/targetsetupwidget.cpp +++ b/src/plugins/projectexplorer/targetsetupwidget.cpp @@ -35,10 +35,13 @@ #include #include + +#include #include #include #include #include +#include #include #include @@ -109,12 +112,6 @@ TargetSetupWidget::TargetSetupWidget(Kit *k, connect(m_manageButton, &QAbstractButton::clicked, this, &TargetSetupWidget::manageKit); } -TargetSetupWidget::~TargetSetupWidget() -{ - qDeleteAll(m_infoList); - m_infoList.clear(); -} - Kit *TargetSetupWidget::kit() { return m_kit; @@ -147,47 +144,46 @@ void TargetSetupWidget::addBuildInfo(BuildInfo *info, bool isImport) { if (isImport && !m_haveImported) { // disable everything on first import - for (int i = 0; i < m_enabled.count(); ++i) { - m_enabled[i] = false; - m_checkboxes[i]->setChecked(false); + for (BuildInfoStore &store : m_infoStore) { + store.isEnabled = false; + store.checkbox->setChecked(false); } m_selected = 0; m_haveImported = true; } - int pos = m_pathChoosers.count(); - m_enabled.append(true); + const int pos = static_cast(m_infoStore.size()); + + BuildInfoStore store; + store.buildInfo = info; + store.isEnabled = true; ++m_selected; - m_infoList << info; + store.checkbox = new QCheckBox; + store.checkbox->setText(info->displayName); + store.checkbox->setChecked(store.isEnabled); + store.checkbox->setAttribute(Qt::WA_LayoutUsesWidgetRect); + m_newBuildsLayout->addWidget(store.checkbox, pos * 2, 0); - auto checkbox = new QCheckBox; - checkbox->setText(info->displayName); - checkbox->setChecked(m_enabled.at(pos)); - checkbox->setAttribute(Qt::WA_LayoutUsesWidgetRect); - m_newBuildsLayout->addWidget(checkbox, pos * 2, 0); + store.pathChooser = new Utils::PathChooser(); + store.pathChooser->setExpectedKind(Utils::PathChooser::Directory); + store.pathChooser->setFileName(info->buildDirectory); + store.pathChooser->setHistoryCompleter(QLatin1String("TargetSetup.BuildDir.History")); + store.pathChooser->setReadOnly(isImport); + m_newBuildsLayout->addWidget(store.pathChooser, pos * 2, 1); - auto pathChooser = new Utils::PathChooser(); - pathChooser->setExpectedKind(Utils::PathChooser::Directory); - pathChooser->setFileName(info->buildDirectory); - pathChooser->setHistoryCompleter(QLatin1String("TargetSetup.BuildDir.History")); - pathChooser->setReadOnly(isImport); - m_newBuildsLayout->addWidget(pathChooser, pos * 2, 1); + store.issuesLabel = new QLabel; + store.issuesLabel->setIndent(32); + m_newBuildsLayout->addWidget(store.issuesLabel, pos * 2 + 1, 0, 1, 2); + store.issuesLabel->setVisible(false); - auto reportIssuesLabel = new QLabel; - reportIssuesLabel->setIndent(32); - m_newBuildsLayout->addWidget(reportIssuesLabel, pos * 2 + 1, 0, 1, 2); - reportIssuesLabel->setVisible(false); + connect(store.checkbox, &QAbstractButton::toggled, this, &TargetSetupWidget::checkBoxToggled); + connect(store.pathChooser, &Utils::PathChooser::rawPathChanged, this, &TargetSetupWidget::pathChanged); - connect(checkbox, &QAbstractButton::toggled, this, &TargetSetupWidget::checkBoxToggled); - connect(pathChooser, &Utils::PathChooser::rawPathChanged, this, &TargetSetupWidget::pathChanged); + store.hasIssues = false; + m_infoStore.emplace_back(std::move(store)); - m_checkboxes.append(checkbox); - m_pathChoosers.append(pathChooser); - m_reportIssuesLabels.append(reportIssuesLabel); - - m_issues.append(false); reportIssues(pos); emit selectedToggled(); @@ -198,16 +194,9 @@ void TargetSetupWidget::targetCheckBoxToggled(bool b) if (m_ignoreChange) return; m_detailsWidget->widget()->setEnabled(b); - if (b) { - foreach (bool error, m_issues) { - if (error) { - m_detailsWidget->setState(Utils::DetailsWidget::Expanded); - break; - } - } - } else { - m_detailsWidget->setState(Utils::DetailsWidget::Collapsed); - } + m_detailsWidget->setState(b && Utils::contains(m_infoStore, &BuildInfoStore::hasIssues) + ? Utils::DetailsWidget::Expanded + : Utils::DetailsWidget::Collapsed); emit selectedToggled(); } @@ -258,26 +247,17 @@ void TargetSetupWidget::handleKitUpdate(Kit *k) QList TargetSetupWidget::selectedBuildInfoList() const { QList result; - for (int i = 0; i < m_infoList.count(); ++i) { - if (m_enabled.at(i)) - result.append(m_infoList.at(i)); + for (const BuildInfoStore &store : m_infoStore) { + if (store.isEnabled) + result.append(store.buildInfo); } return result; } void TargetSetupWidget::clear() { - qDeleteAll(m_checkboxes); - m_checkboxes.clear(); - qDeleteAll(m_pathChoosers); - m_pathChoosers.clear(); - qDeleteAll(m_reportIssuesLabels); - m_reportIssuesLabels.clear(); - qDeleteAll(m_infoList); - m_infoList.clear(); + m_infoStore.clear(); - m_issues.clear(); - m_enabled.clear(); m_selected = 0; m_haveImported = false; @@ -289,13 +269,13 @@ void TargetSetupWidget::checkBoxToggled(bool b) auto box = qobject_cast(sender()); if (!box) return; - int index = m_checkboxes.indexOf(box); - if (index == -1) - return; - if (m_enabled[index] == b) + auto it = std::find_if(m_infoStore.begin(), m_infoStore.end(), + [box](const BuildInfoStore &store) { return store.checkbox == box; }); + QTC_ASSERT(it != m_infoStore.end(), return); + if (it->isEnabled == b) return; m_selected += b ? 1 : -1; - m_enabled[index] = b; + it->isEnabled = b; if ((m_selected == 0 && !b) || (m_selected == 1 && b)) { emit selectedToggled(); m_detailsWidget->setChecked(b); @@ -307,23 +287,27 @@ void TargetSetupWidget::pathChanged() if (m_ignoreChange) return; auto pathChooser = qobject_cast(sender()); - if (!pathChooser) - return; - int index = m_pathChoosers.indexOf(pathChooser); - if (index == -1) - return; - m_infoList[index]->buildDirectory = pathChooser->fileName(); - reportIssues(index); + QTC_ASSERT(pathChooser, return); + + auto it = std::find_if(m_infoStore.begin(), m_infoStore.end(), + [pathChooser](const BuildInfoStore &store) { + return store.pathChooser == pathChooser; + }); + QTC_ASSERT(it != m_infoStore.end(), return); + it->buildInfo->buildDirectory = pathChooser->fileName(); + reportIssues(static_cast(std::distance(m_infoStore.begin(), it))); } void TargetSetupWidget::reportIssues(int index) { - QPair issues = findIssues(m_infoList.at(index)); - QLabel *reportIssuesLabel = m_reportIssuesLabels.at(index); - reportIssuesLabel->setText(issues.second); - bool error = issues.first != Task::Unknown; - reportIssuesLabel->setVisible(error); - m_issues[index] = error; + const int size = static_cast(m_infoStore.size()); + QTC_ASSERT(index >= 0 && index < size, return); + + BuildInfoStore &store = m_infoStore[static_cast(index)]; + QPair issues = findIssues(store.buildInfo); + store.issuesLabel->setText(issues.second); + store.hasIssues = issues.first != Task::Unknown; + store.issuesLabel->setVisible(store.hasIssues); } QPair TargetSetupWidget::findIssues(const BuildInfo *info) @@ -356,5 +340,25 @@ QPair TargetSetupWidget::findIssues(const BuildInfo *in return qMakePair(highestType, text); } +TargetSetupWidget::BuildInfoStore::~BuildInfoStore() +{ + delete buildInfo; + delete checkbox; + delete label; + delete issuesLabel; + delete pathChooser; +} + +TargetSetupWidget::BuildInfoStore::BuildInfoStore(TargetSetupWidget::BuildInfoStore &&other) +{ + std::swap(other.buildInfo, buildInfo); + std::swap(other.checkbox, checkbox); + std::swap(other.label, label); + std::swap(other.issuesLabel, issuesLabel); + std::swap(other.pathChooser, pathChooser); + std::swap(other.isEnabled, isEnabled); + std::swap(other.hasIssues, hasIssues); +} + } // namespace Internal } // namespace ProjectExplorer diff --git a/src/plugins/projectexplorer/targetsetupwidget.h b/src/plugins/projectexplorer/targetsetupwidget.h index c3d83d55a7e..63237ae312f 100644 --- a/src/plugins/projectexplorer/targetsetupwidget.h +++ b/src/plugins/projectexplorer/targetsetupwidget.h @@ -60,7 +60,6 @@ public: TargetSetupWidget(Kit *k, const QString &projectPath, const QList &infoList); - ~TargetSetupWidget() override; Kit *kit(); void clearKit(); @@ -95,14 +94,27 @@ private: Utils::DetailsWidget *m_detailsWidget; QPushButton *m_manageButton; QGridLayout *m_newBuildsLayout; - QList m_checkboxes; - QList m_pathChoosers; - QList m_infoList; - QList m_enabled; - QList m_reportIssuesLabels; - QList m_issues; + + struct BuildInfoStore { + ~BuildInfoStore(); + BuildInfoStore() = default; + BuildInfoStore(const BuildInfoStore &other) = delete; + BuildInfoStore(BuildInfoStore &&other); + BuildInfoStore &operator=(const BuildInfoStore &other) = delete; + BuildInfoStore &operator=(BuildInfoStore &&other) = delete; + + BuildInfo *buildInfo = nullptr; + QCheckBox *checkbox = nullptr; + QLabel *label = nullptr; + QLabel *issuesLabel = nullptr; + Utils::PathChooser *pathChooser = nullptr; + bool isEnabled = false; + bool hasIssues = false; + }; + std::vector m_infoStore; + bool m_ignoreChange = false; - int m_selected = 0; // Number of selected buildconfiguartions + int m_selected = 0; // Number of selected "buildconfiguartions" }; } // namespace Internal