ProjectExplorer: Introduce and use a new MultiSelectionAspect

Only provide the minimum functionality to make it usable
for the target selection in Make steps.

Task-number: QTCREATORBUG-25088
Change-Id: I6458a0bd544d2fff596df3240c69e05d7bdea14b
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

src/libs/utils/aspects.cpp

@@ -40,6 +40,7 @@
 #include <QFormLayout>
 #include <QLabel>
 #include <QLineEdit>
+#include <QListWidget>
 #include <QPointer>
 #include <QRadioButton>
 #include <QSpinBox>
@@ -250,6 +251,23 @@ public:
     QString m_tooltip;
 };
 
+class MultiSelectionAspectPrivate
+{
+public:
+    QStringList m_value;
+    QStringList m_allValues;
+    MultiSelectionAspect::DisplayStyle m_displayStyle
+        = MultiSelectionAspect::DisplayStyle::ListView;
+    QString m_labelText;
+
+    // These are all owned by the configuration widget.
+    QPointer<QListWidget> m_listView;
+    QPointer<QLabel> m_label;
+
+    void updateListView();
+    bool setValueSelectedHelper(const QString &value, bool on);
+};
+
 class StringAspectPrivate
 {
 public:
@@ -1033,6 +1051,129 @@ void SelectionAspect::addOption(const QString &displayName, const QString &toolT
     d->m_options.append({displayName, toolTip});
 }
 
+/*!
+    \class Utils::MultiSelectionAspect
+    \inmodule QtCreator
+
+    \brief A multi-selection aspect represents one or more choices out of
+    several.
+
+    The multi-selection aspect is displayed using a QListWidget with
+    checkable items.
+*/
+
+MultiSelectionAspect::MultiSelectionAspect()
+    : d(new Internal::MultiSelectionAspectPrivate)
+{}
+
+/*!
+    \reimp
+*/
+MultiSelectionAspect::~MultiSelectionAspect() = default;
+
+/*!
+    \reimp
+*/
+void MultiSelectionAspect::addToLayout(LayoutBuilder &builder)
+{
+    QTC_CHECK(d->m_listView == nullptr);
+    if (d->m_allValues.isEmpty())
+        return;
+
+    switch (d->m_displayStyle) {
+    case DisplayStyle::ListView:
+        d->m_label = new QLabel(d->m_labelText);
+        d->m_listView = new QListWidget;
+        for (const QString &value : qAsConst(d->m_allValues)) {
+            auto item = new QListWidgetItem(value, d->m_listView);
+            item->setFlags(item->flags() | Qt::ItemIsUserCheckable);
+            item->setCheckState(d->m_value.contains(item->text()) ? Qt::Checked : Qt::Unchecked);
+        }
+        connect(d->m_listView, &QListWidget::itemChanged, this,
+            [this](QListWidgetItem *item) {
+            if (d->setValueSelectedHelper(item->text(), item->checkState() & Qt::Checked))
+                emit changed();
+        });
+        builder.addItems({d->m_label.data(), d->m_listView.data()});
+    }
+}
+
+bool Internal::MultiSelectionAspectPrivate::setValueSelectedHelper(const QString &value, bool on)
+{
+    if (on && !m_value.contains(value)) {
+        m_value.append(value);
+        return true;
+    }
+    if (!on && m_value.contains(value)) {
+        m_value.removeOne(value);
+        return true;
+    }
+    return false;
+}
+
+QStringList MultiSelectionAspect::allValues() const
+{
+    return d->m_allValues;
+}
+
+void MultiSelectionAspect::setAllValues(const QStringList &val)
+{
+    d->m_allValues = val;
+}
+
+void MultiSelectionAspect::setLabelText(const QString &labelText)
+{
+    d->m_labelText = labelText;
+}
+
+void Internal::MultiSelectionAspectPrivate::updateListView()
+{
+    if (!m_listView)
+        return;
+    const int n = m_listView->count();
+    QTC_CHECK(n == m_allValues.size());
+    for (int i = 0; i != n; ++i) {
+        auto item = m_listView->item(i);
+        item->setCheckState(m_value.contains(item->text()) ? Qt::Checked : Qt::Unchecked);
+    }
+}
+
+/*!
+    \reimp
+*/
+void MultiSelectionAspect::fromMap(const QVariantMap &map)
+{
+    d->m_value = map.value(settingsKey(), QStringList()).toStringList();
+}
+
+/*!
+    \reimp
+*/
+void MultiSelectionAspect::toMap(QVariantMap &data) const
+{
+    saveToMap(data, d->m_value, QStringList());
+}
+
+void MultiSelectionAspect::setDisplayStyle(MultiSelectionAspect::DisplayStyle style)
+{
+    d->m_displayStyle = style;
+}
+
+QStringList MultiSelectionAspect::value() const
+{
+    return d->m_value;
+}
+
+void MultiSelectionAspect::setValue(const QStringList &value)
+{
+    if (d->m_value == value)
+        return;
+    d->m_value = value;
+    d->updateListView();
+    emit changed();
+}
+
+
 /*!
     \class Utils::IntegerAspect
     \inmodule QtCreator

src/libs/utils/aspects.h

@@ -44,6 +44,7 @@ namespace Internal {
 class AspectContainerPrivate;
 class BoolAspectPrivate;
 class IntegerAspectPrivate;
+class MultiSelectionAspectPrivate;
 class SelectionAspectPrivate;
 class StringAspectPrivate;
 class StringListAspectPrivate;
@@ -97,7 +98,6 @@ protected:
 
 class QTCREATOR_UTILS_EXPORT BaseAspects
 {
-
     BaseAspects(const BaseAspects &) = delete;
     BaseAspects &operator=(const BaseAspects &) = delete;
 
@@ -202,6 +202,34 @@ private:
     std::unique_ptr<Internal::SelectionAspectPrivate> d;
 };
 
+class QTCREATOR_UTILS_EXPORT MultiSelectionAspect : public BaseAspect
+{
+    Q_OBJECT
+
+public:
+    MultiSelectionAspect();
+    ~MultiSelectionAspect() override;
+
+    void addToLayout(LayoutBuilder &builder) override;
+
+    enum class DisplayStyle { ListView };
+    void setDisplayStyle(DisplayStyle style);
+
+    QStringList value() const;
+    void setValue(const QStringList &val);
+
+    QStringList allValues() const;
+    void setAllValues(const QStringList &val);
+
+    void setLabelText(const QString &labelText);
+
+    void fromMap(const QVariantMap &map) override;
+    void toMap(QVariantMap &map) const override;
+
+private:
+    std::unique_ptr<Internal::MultiSelectionAspectPrivate> d;
+};
+
 class QTCREATOR_UTILS_EXPORT StringAspect : public BaseAspect
 {
     Q_OBJECT

src/plugins/autotoolsprojectmanager/makestep.cpp

@@ -49,10 +49,10 @@ MakeStep::MakeStep(ProjectExplorer::BuildStepList *bsl, Utils::Id id)
 {
     setAvailableBuildTargets({"all", "clean"});
     if (bsl->id() == ProjectExplorer::Constants::BUILDSTEPS_CLEAN) {
-        setBuildTarget("clean", true);
+        setSelectedBuildTarget("clean");
         setIgnoreReturnValue(true);
     } else {
-        setBuildTarget("all", true);
+        setSelectedBuildTarget("all");
     }
 }
 

src/plugins/genericprojectmanager/genericmakestep.cpp

@@ -43,13 +43,13 @@ public:
 GenericMakeStep::GenericMakeStep(BuildStepList *parent, Utils::Id id)
     : MakeStep(parent, id)
 {
+    setAvailableBuildTargets({"all", "clean"});
     if (parent->id() == ProjectExplorer::Constants::BUILDSTEPS_BUILD) {
-        setBuildTarget("all");
+        setSelectedBuildTarget("all");
     } else if (parent->id() == ProjectExplorer::Constants::BUILDSTEPS_CLEAN) {
-        setBuildTarget("clean");
+        setSelectedBuildTarget("clean");
         setIgnoreReturnValue(true);
     }
-    setAvailableBuildTargets({"all", "clean"});
 }
 
 GenericMakeStepFactory::GenericMakeStepFactory()

src/plugins/projectexplorer/makestep.cpp

@@ -49,8 +49,6 @@
 #include <QFormLayout>
 #include <QLabel>
 #include <QLineEdit>
-#include <QListWidget>
-#include <QSpinBox>
 #include <QThread>
 
 using namespace Core;
@@ -105,8 +103,9 @@ MakeStep::MakeStep(BuildStepList *parent, Id id)
          .arg(text) + "</p></body></html>");
     m_nonOverrideWarning->setIconType(InfoLabel::Warning);
 
-    m_buildTargetsAspect = addAspect<StringListAspect>();
+    m_buildTargetsAspect = addAspect<MultiSelectionAspect>();
     m_buildTargetsAspect->setSettingsKey(id.withSuffix(BUILD_TARGETS_SUFFIX).toString());
+    m_buildTargetsAspect->setLabelText(tr("Targets:"));
 
     const auto updateMakeLabel = [this] {
         const QString defaultMake = defaultMakeCommand().toString();
@@ -121,15 +120,14 @@ MakeStep::MakeStep(BuildStepList *parent, Id id)
     connect(m_makeCommandAspect, &StringAspect::changed, this, updateMakeLabel);
 }
 
-void MakeStep::setBuildTarget(const QString &buildTarget)
+void MakeStep::setSelectedBuildTarget(const QString &buildTarget)
 {
-    if (!buildTarget.isEmpty())
+    m_buildTargetsAspect->setValue({buildTarget});
-        setBuildTarget(buildTarget, true);
 }
 
 void MakeStep::setAvailableBuildTargets(const QStringList &buildTargets)
 {
-    m_availableTargets = buildTargets;
+    m_buildTargetsAspect->setAllValues(buildTargets);
 }
 
 bool MakeStep::init()
@@ -342,11 +340,6 @@ QWidget *MakeStep::createConfigWidget()
 {
     auto widget = new QWidget;
 
-    auto targetsLabel = new QLabel(widget);
-    targetsLabel->setText(tr("Targets:"));
-
-    auto targetsList = new QListWidget(widget);
-
     auto disableInSubDirsLabel = new QLabel(tr("Disable in subdirectories:"), widget);
     auto disableInSubDirsCheckBox = new QCheckBox(widget);
     disableInSubDirsCheckBox->setToolTip(tr("Runs this step only for a top-level build."));
@@ -356,7 +349,7 @@ QWidget *MakeStep::createConfigWidget()
     builder.addRow(m_userArgumentsAspect);
     builder.addRow(m_jobCountContainer);
     builder.addRow({disableInSubDirsLabel, disableInSubDirsCheckBox});
-    builder.addRow({targetsLabel, targetsList});
+    builder.addRow(m_buildTargetsAspect);
 
     if (!m_disablingForSubDirsSupported) {
         disableInSubDirsLabel->hide();
@@ -367,16 +360,6 @@ QWidget *MakeStep::createConfigWidget()
         });
     }
 
-    for (const QString &target : qAsConst(m_availableTargets)) {
-        auto item = new QListWidgetItem(target, targetsList);
-        item->setFlags(item->flags() | Qt::ItemIsUserCheckable);
-        item->setCheckState(buildsTarget(item->text()) ? Qt::Checked : Qt::Unchecked);
-    }
-    if (m_availableTargets.isEmpty()) {
-        targetsLabel->hide();
-        targetsList->hide();
-    }
-
     VariableChooser::addSupportForChildWidgets(widget, macroExpander());
 
     setSummaryUpdater([this] {
@@ -420,12 +403,7 @@ QWidget *MakeStep::createConfigWidget()
     connect(m_userArgumentsAspect, &StringAspect::changed, widget, updateDetails);
     connect(m_userJobCountAspect, &IntegerAspect::changed, widget, updateDetails);
     connect(m_overrideMakeflagsAspect, &BoolAspect::changed, widget, updateDetails);
-
+    connect(m_buildTargetsAspect, &BaseAspect::changed, widget, updateDetails);
-    connect(targetsList, &QListWidget::itemChanged, this,
-            [this, updateDetails](QListWidgetItem *item) {
-        setBuildTarget(item->text(), item->checkState() & Qt::Checked);
-        updateDetails();
-    });
 
     connect(ProjectExplorerPlugin::instance(), &ProjectExplorerPlugin::settingsChanged,
             widget, updateDetails);
@@ -439,25 +417,9 @@ QWidget *MakeStep::createConfigWidget()
     return widget;
 }
 
-bool MakeStep::buildsTarget(const QString &target) const
-{
-    return m_buildTargetsAspect->value().contains(target);
-}
-
-void MakeStep::setBuildTarget(const QString &target, bool on)
-{
-    QStringList old = m_buildTargetsAspect->value();
-    if (on && !old.contains(target))
-         old << target;
-    else if (!on && old.contains(target))
-        old.removeOne(target);
-
-    m_buildTargetsAspect->setValue(old);
-}
-
 QStringList MakeStep::availableTargets() const
 {
-    return m_availableTargets;
+    return m_buildTargetsAspect->allValues();
 }
 
 } // namespace ProjectExplorer

src/plugins/projectexplorer/makestep.h

@@ -45,14 +45,13 @@ public:
     };
     explicit MakeStep(ProjectExplorer::BuildStepList *parent, Utils::Id id);
 
-    void setBuildTarget(const QString &buildTarget);
     void setAvailableBuildTargets(const QStringList &buildTargets);
+    void setSelectedBuildTarget(const QString &buildTarget);
 
     bool init() override;
     void setupOutputFormatter(Utils::OutputFormatter *formatter) override;
     QWidget *createConfigWidget() override;
-    bool buildsTarget(const QString &target) const;
+
-    void setBuildTarget(const QString &target, bool on);
     QStringList availableTargets() const;
     QString userArguments() const;
     void setUserArguments(const QString &args);
@@ -87,8 +86,7 @@ private:
     static int defaultJobCount();
     QStringList jobArguments() const;
 
-    Utils::StringListAspect *m_buildTargetsAspect = nullptr;
+    Utils::MultiSelectionAspect *m_buildTargetsAspect = nullptr;
-    QStringList m_availableTargets;
     Utils::StringAspect *m_makeCommandAspect = nullptr;
     Utils::StringAspect *m_userArgumentsAspect = nullptr;
     Utils::AspectContainer *m_jobCountContainer = nullptr;