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 <hjk@qt.io>

src/plugins/projectexplorer/targetsetupwidget.cpp

@@ -35,10 +35,13 @@
 
 #include <coreplugin/icore.h>
 #include <extensionsystem/pluginmanager.h>
+
+#include <utils/algorithm.h>
 #include <utils/detailsbutton.h>
 #include <utils/detailswidget.h>
 #include <utils/hostosinfo.h>
 #include <utils/pathchooser.h>
+#include <utils/qtcassert.h>
 
 #include <QCheckBox>
 #include <QHBoxLayout>
@@ -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) {
+        for (BuildInfoStore &store : m_infoStore) {
-            m_enabled[i] = false;
+            store.isEnabled = false;
-            m_checkboxes[i]->setChecked(false);
+            store.checkbox->setChecked(false);
         }
         m_selected = 0;
 
         m_haveImported = true;
     }
 
-    int pos = m_pathChoosers.count();
+    const int pos = static_cast<int>(m_infoStore.size());
-    m_enabled.append(true);
+
+    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;
+    store.pathChooser = new Utils::PathChooser();
-    checkbox->setText(info->displayName);
+    store.pathChooser->setExpectedKind(Utils::PathChooser::Directory);
-    checkbox->setChecked(m_enabled.at(pos));
+    store.pathChooser->setFileName(info->buildDirectory);
-    checkbox->setAttribute(Qt::WA_LayoutUsesWidgetRect);
+    store.pathChooser->setHistoryCompleter(QLatin1String("TargetSetup.BuildDir.History"));
-    m_newBuildsLayout->addWidget(checkbox, pos * 2, 0);
+    store.pathChooser->setReadOnly(isImport);
+    m_newBuildsLayout->addWidget(store.pathChooser, pos * 2, 1);
 
-    auto pathChooser = new Utils::PathChooser();
+    store.issuesLabel = new QLabel;
-    pathChooser->setExpectedKind(Utils::PathChooser::Directory);
+    store.issuesLabel->setIndent(32);
-    pathChooser->setFileName(info->buildDirectory);
+    m_newBuildsLayout->addWidget(store.issuesLabel, pos * 2 + 1, 0, 1, 2);
-    pathChooser->setHistoryCompleter(QLatin1String("TargetSetup.BuildDir.History"));
+    store.issuesLabel->setVisible(false);
-    pathChooser->setReadOnly(isImport);
-    m_newBuildsLayout->addWidget(pathChooser, pos * 2, 1);
 
-    auto reportIssuesLabel = new QLabel;
+    connect(store.checkbox, &QAbstractButton::toggled, this, &TargetSetupWidget::checkBoxToggled);
-    reportIssuesLabel->setIndent(32);
+    connect(store.pathChooser, &Utils::PathChooser::rawPathChanged, this, &TargetSetupWidget::pathChanged);
-    m_newBuildsLayout->addWidget(reportIssuesLabel, pos * 2 + 1, 0, 1, 2);
-    reportIssuesLabel->setVisible(false);
 
-    connect(checkbox, &QAbstractButton::toggled, this, &TargetSetupWidget::checkBoxToggled);
+    store.hasIssues = false;
-    connect(pathChooser, &Utils::PathChooser::rawPathChanged, this, &TargetSetupWidget::pathChanged);
+    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) {
+    m_detailsWidget->setState(b && Utils::contains(m_infoStore, &BuildInfoStore::hasIssues)
-        foreach (bool error, m_issues) {
+                              ? Utils::DetailsWidget::Expanded
-            if (error) {
+                              : Utils::DetailsWidget::Collapsed);
-                m_detailsWidget->setState(Utils::DetailsWidget::Expanded);
-                break;
-            }
-        }
-    } else {
-        m_detailsWidget->setState(Utils::DetailsWidget::Collapsed);
-    }
     emit selectedToggled();
 }
 
@@ -258,26 +247,17 @@ void TargetSetupWidget::handleKitUpdate(Kit *k)
 QList<const BuildInfo *> TargetSetupWidget::selectedBuildInfoList() const
 {
     QList<const BuildInfo *> result;
-    for (int i = 0; i < m_infoList.count(); ++i) {
+    for (const BuildInfoStore &store : m_infoStore) {
-        if (m_enabled.at(i))
+        if (store.isEnabled)
-            result.append(m_infoList.at(i));
+            result.append(store.buildInfo);
     }
     return result;
 }
 
 void TargetSetupWidget::clear()
 {
-    qDeleteAll(m_checkboxes);
+    m_infoStore.clear();
-    m_checkboxes.clear();
-    qDeleteAll(m_pathChoosers);
-    m_pathChoosers.clear();
-    qDeleteAll(m_reportIssuesLabels);
-    m_reportIssuesLabels.clear();
-    qDeleteAll(m_infoList);
-    m_infoList.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<QCheckBox *>(sender());
     if (!box)
         return;
-    int index = m_checkboxes.indexOf(box);
+    auto it = std::find_if(m_infoStore.begin(), m_infoStore.end(),
-    if (index == -1)
+                           [box](const BuildInfoStore &store) { return store.checkbox == box; });
-        return;
+    QTC_ASSERT(it != m_infoStore.end(), return);
-    if (m_enabled[index] == b)
+    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<Utils::PathChooser *>(sender());
-    if (!pathChooser)
+    QTC_ASSERT(pathChooser, return);
-        return;
+
-    int index = m_pathChoosers.indexOf(pathChooser);
+    auto it = std::find_if(m_infoStore.begin(), m_infoStore.end(),
-    if (index == -1)
+                           [pathChooser](const BuildInfoStore &store) {
-        return;
+        return store.pathChooser == pathChooser;
-    m_infoList[index]->buildDirectory = pathChooser->fileName();
+    });
-    reportIssues(index);
+    QTC_ASSERT(it != m_infoStore.end(), return);
+    it->buildInfo->buildDirectory = pathChooser->fileName();
+    reportIssues(static_cast<int>(std::distance(m_infoStore.begin(), it)));
 }
 
 void TargetSetupWidget::reportIssues(int index)
 {
-    QPair<Task::TaskType, QString> issues = findIssues(m_infoList.at(index));
+    const int size = static_cast<int>(m_infoStore.size());
-    QLabel *reportIssuesLabel = m_reportIssuesLabels.at(index);
+    QTC_ASSERT(index >= 0 && index < size, return);
-    reportIssuesLabel->setText(issues.second);
+
-    bool error = issues.first != Task::Unknown;
+    BuildInfoStore &store = m_infoStore[static_cast<size_t>(index)];
-    reportIssuesLabel->setVisible(error);
+    QPair<Task::TaskType, QString> issues = findIssues(store.buildInfo);
-    m_issues[index] = error;
+    store.issuesLabel->setText(issues.second);
+    store.hasIssues = issues.first != Task::Unknown;
+    store.issuesLabel->setVisible(store.hasIssues);
 }
 
 QPair<Task::TaskType, QString> TargetSetupWidget::findIssues(const BuildInfo *info)
@@ -356,5 +340,25 @@ QPair<Task::TaskType, QString> 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

src/plugins/projectexplorer/targetsetupwidget.h

@@ -60,7 +60,6 @@ public:
     TargetSetupWidget(Kit *k,
                       const QString &projectPath,
                       const QList<BuildInfo *> &infoList);
-    ~TargetSetupWidget() override;
 
     Kit *kit();
     void clearKit();
@@ -95,14 +94,27 @@ private:
     Utils::DetailsWidget *m_detailsWidget;
     QPushButton *m_manageButton;
     QGridLayout *m_newBuildsLayout;
-    QList<QCheckBox *> m_checkboxes;
+
-    QList<Utils::PathChooser *> m_pathChoosers;
+    struct BuildInfoStore {
-    QList<BuildInfo *> m_infoList;
+        ~BuildInfoStore();
-    QList<bool> m_enabled;
+        BuildInfoStore() = default;
-    QList<QLabel *> m_reportIssuesLabels;
+        BuildInfoStore(const BuildInfoStore &other) = delete;
-    QList<bool> m_issues;
+        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<BuildInfoStore> m_infoStore;
+
     bool m_ignoreChange = false;
-    int m_selected = 0; // Number of selected buildconfiguartions
+    int m_selected = 0; // Number of selected "buildconfiguartions"
 };
 
 } // namespace Internal