CMake: Fix several issues with the CMakeBuildStep

- Store the selection of the "Current executable" target again
  (broke apparently with 2c822ae3)
- Display the resolved target of the "Current executable" seletion
  in the command line, instead of the fixed "
   '<Current executable>' text
- Make the "Current executable" translatable
- Add a tooltip explaining what it is
- Use a Utils::TreeModel instead of a QStandardItemModel for the
  target model
- As side-effect, searching in the target view using Ctrl-F seems
  to magically work again.

Change-Id: Ia4d0913f6e586f49f74da66651a9177437dad6d9
Reviewed-by: Cristian Adam <cristian.adam@qt.io>

2 files changed, 186 insertions, 202 deletions

diff --git a/src/plugins/cmakeprojectmanager/cmakebuildstep.cpp b/src/plugins/cmakeprojectmanager/cmakebuildstep.cpp
index 8fd69fec0ef..eb721167cd5 100644
--- a/src/plugins/cmakeprojectmanager/cmakebuildstep.cpp
+++ b/src/plugins/cmakeprojectmanager/cmakebuildstep.cpp
@@ -46,7 +46,9 @@
 #include <QBoxLayout>
 #include <QListWidget>
 #include <QRegularExpression>
+#include <QTreeView>
 
+using namespace Core;
 using namespace ProjectExplorer;
 using namespace Utils;
 
@@ -56,8 +58,8 @@ namespace Internal {
 const char BUILD_TARGETS_KEY[] = "CMakeProjectManager.MakeStep.BuildTargets";
 const char CMAKE_ARGUMENTS_KEY[] = "CMakeProjectManager.MakeStep.CMakeArguments";
 const char TOOL_ARGUMENTS_KEY[] = "CMakeProjectManager.MakeStep.AdditionalArguments";
-const char ADD_RUNCONFIGURATION_ARGUMENT_KEY[] = "CMakeProjectManager.MakeStep.AddRunConfigurationArgument";
-const char ADD_RUNCONFIGURATION_TEXT[] = "Current executable";
+
+// CmakeProgressParser
 
 class CmakeProgressParser : public Utils::OutputLineParser
 {
@@ -106,28 +108,63 @@ private:
     bool m_useNinja = false;
 };
 
-class CMakeBuildStepConfigWidget : public BuildStepConfigWidget
+
+// CmakeTargetItem
+
+CMakeTargetItem::CMakeTargetItem(const QString &target, CMakeBuildStep *step, bool special)
+    : m_target(target), m_step(step), m_special(special)
 {
-    Q_DECLARE_TR_FUNCTIONS(CMakeProjectManager::Internal::CMakeBuildStepConfigWidget)
+}
 
-public:
-    explicit CMakeBuildStepConfigWidget(CMakeBuildStep *buildStep);
+QVariant CMakeTargetItem::data(int column, int role) const
+{
+    if (column == 0) {
+        if (role == Qt::DisplayRole) {
+            if (m_target.isEmpty())
+                return CMakeBuildStep::tr("Current executable");
+            return m_target;
+        }
 
-private:
-    void itemsChanged();
-    void updateDetails();
-    void buildTargetsChanged();
-    void updateBuildTargets();
+        if (role == Qt::ToolTipRole) {
+            if (m_target.isEmpty()) {
+                return CMakeBuildStep::tr("Build the executable used in the active Run "
+                                          "configuration. Currently: %1")
+                        .arg(m_step->activeRunConfigTarget());
+            }
+            return CMakeBuildStep::tr("Target: %1").arg(m_target);
+        }
 
-    CMakeBuildStep *m_buildStep;
-    QListWidget *m_buildTargetsList;
-};
+        if (role == Qt::CheckStateRole)
+            return m_step->buildsBuildTarget(m_target) ? Qt::Checked : Qt::Unchecked;
 
-static bool isCurrentExecutableTarget(const QString &target)
+        if (role == Qt::FontRole) {
+            if (m_special) {
+                QFont italics;
+                italics.setItalic(true);
+                return italics;
+            }
+        }
+    }
+
+    return QVariant();
+}
+
+bool CMakeTargetItem::setData(int column, const QVariant &data, int role)
 {
-    return target == ADD_RUNCONFIGURATION_TEXT;
+    if (column == 0 && role == Qt::CheckStateRole) {
+        m_step->setBuildsBuildTarget(m_target, data.value<Qt::CheckState>() == Qt::Checked);
+        return true;
+    }
+    return TreeItem::setData(column, data, role);
 }
 
+Qt::ItemFlags CMakeTargetItem::flags(int) const
+{
+    return Qt::ItemIsUserCheckable | Qt::ItemIsEnabled | Qt::ItemIsSelectable;
+}
+
+// CMakeBuildStep
+
 CMakeBuildStep::CMakeBuildStep(BuildStepList *bsl, Utils::Id id) :
     AbstractProcessStep(bsl, id)
 {
@@ -144,9 +181,9 @@ CMakeBuildStep::CMakeBuildStep(BuildStepList *bsl, Utils::Id id) :
     m_toolArguments->setLabelText(tr("Tool arguments:"));
     m_toolArguments->setDisplayStyle(StringAspect::LineEditDisplay);
 
-    // Set a good default build target:
-    if (m_buildTargets.isEmpty())
-        setBuildTargets({defaultBuildTarget()});
+    m_buildTargetModel.setHeader({tr("Target")});
+
+    setBuildTargets({defaultBuildTarget()});
 
     setLowPriority();
 
@@ -157,51 +194,39 @@ CMakeBuildStep::CMakeBuildStep(BuildStepList *bsl, Utils::Id id) :
             env.set("NINJA_STATUS", ninjaProgressString + "%o/sec] ");
     });
 
-    connect(target(), &Target::parsingFinished,
-            this, &CMakeBuildStep::handleBuildTargetsChanges);
-}
-
-void CMakeBuildStep::handleBuildTargetsChanges(bool success)
-{
-    if (!success)
-        return; // Do not change when parsing failed.
-    const QStringList results = Utils::filtered(m_buildTargets, [this](const QString &s) {
-        return knownBuildTargets().contains(s);
+    connect(target(), &Target::parsingFinished, this, [this](bool success) {
+        if (success) // Do not change when parsing failed.
+            recreateBuildTargetsModel();
     });
-    if (results.isEmpty())
-        setBuildTargets({defaultBuildTarget()});
-    else {
-        setBuildTargets(results);
-    }
-    emit buildTargetsChanged();
+
+    connect(target(), &Target::activeRunConfigurationChanged,
+            this, &CMakeBuildStep::updateBuildTargetsModel);
 }
 
+
 QVariantMap CMakeBuildStep::toMap() const
 {
     QVariantMap map(AbstractProcessStep::toMap());
-    // Use QStringList for compatibility with old files
-    map.insert(BUILD_TARGETS_KEY, QStringList(m_buildTargets));
+    map.insert(BUILD_TARGETS_KEY, m_buildTargets);
     return map;
 }
 
 bool CMakeBuildStep::fromMap(const QVariantMap &map)
 {
-    m_buildTargets = map.value(BUILD_TARGETS_KEY).toStringList();
-    if (map.value(ADD_RUNCONFIGURATION_ARGUMENT_KEY, false).toBool())
-        m_buildTargets = QStringList(ADD_RUNCONFIGURATION_TEXT);
-
+    setBuildTargets(map.value(BUILD_TARGETS_KEY).toStringList());
     return BuildStep::fromMap(map);
 }
 
 bool CMakeBuildStep::init()
 {
-    bool canInit = true;
     BuildConfiguration *bc = buildConfiguration();
     QTC_ASSERT(bc, return false);
+
     if (!bc->isEnabled()) {
         emit addTask(BuildSystemTask(Task::Error,
                                      tr("The build configuration is currently disabled.")));
-        canInit = false;
+        emitFaultyConfigurationMessage();
+        return false;
     }
 
     CMakeTool *tool = CMakeKitAspect::cmakeTool(kit());
@@ -209,23 +234,21 @@ bool CMakeBuildStep::init()
         emit addTask(BuildSystemTask(Task::Error,
                           tr("A CMake tool must be set up for building. "
                              "Configure a CMake tool in the kit options.")));
-        canInit = false;
+        emitFaultyConfigurationMessage();
+        return false;
     }
 
-    RunConfiguration *rc =  target()->activeRunConfiguration();
-    const bool buildCurrent = Utils::contains(m_buildTargets, [](const QString &s) { return isCurrentExecutableTarget(s); });
-    if (buildCurrent && (!rc || rc->buildKey().isEmpty())) {
-        emit addTask(BuildSystemTask(Task::Error,
-                          QCoreApplication::translate("ProjectExplorer::Task",
+    if (m_buildTargets.contains(QString())) {
+        RunConfiguration *rc = target()->activeRunConfiguration();
+        if (!rc || rc->buildKey().isEmpty()) {
+            emit addTask(BuildSystemTask(Task::Error,
+                                         QCoreApplication::translate("ProjectExplorer::Task",
                                     "You asked to build the current Run Configuration's build target only, "
                                     "but it is not associated with a build target. "
                                     "Update the Make Step in your build settings.")));
-        canInit = false;
-    }
-
-    if (!canInit) {
-        emitFaultyConfigurationMessage();
-        return false;
+            emitFaultyConfigurationMessage();
+            return false;
+        }
     }
 
     // Warn if doing out-of-source builds with a CMakeCache.txt is the source directory
@@ -244,7 +267,7 @@ bool CMakeBuildStep::init()
 
     ProcessParameters *pp = processParameters();
     setupProcessParameters(pp);
-    pp->setCommandLine(cmakeCommand(rc));
+    pp->setCommandLine(cmakeCommand());
 
     return AbstractProcessStep::init();
 }
@@ -308,11 +331,6 @@ void CMakeBuildStep::handleProjectWasParsed(bool success)
     }
 }
 
-BuildStepConfigWidget *CMakeBuildStep::createConfigWidget()
-{
-    return new CMakeBuildStepConfigWidget(this);
-}
-
 QString CMakeBuildStep::defaultBuildTarget() const
 {
     const BuildStepList *const bsl = stepList();
@@ -335,15 +353,26 @@ bool CMakeBuildStep::buildsBuildTarget(const QString &target) const
     return m_buildTargets.contains(target);
 }
 
+void CMakeBuildStep::setBuildsBuildTarget(const QString &target, bool on)
+{
+    QStringList targets = m_buildTargets;
+    if (on && !m_buildTargets.contains(target))
+        targets.append(target);
+    if (!on)
+        targets.removeAll(target);
+    setBuildTargets(targets);
+}
+
 void CMakeBuildStep::setBuildTargets(const QStringList &buildTargets)
 {
-    if (m_buildTargets == buildTargets)
-        return;
-    m_buildTargets = buildTargets;
-    emit targetsToBuildChanged();
+    if (buildTargets.isEmpty())
+        m_buildTargets = QStringList(defaultBuildTarget());
+    else
+        m_buildTargets = buildTargets;
+    updateBuildTargetsModel();
 }
 
-Utils::CommandLine CMakeBuildStep::cmakeCommand(RunConfiguration *rc) const
+CommandLine CMakeBuildStep::cmakeCommand() const
 {
     CMakeTool *tool = CMakeKitAspect::cmakeTool(kit());
 
@@ -351,20 +380,12 @@ Utils::CommandLine CMakeBuildStep::cmakeCommand(RunConfiguration *rc) const
     cmd.addArgs({"--build", "."});
 
     cmd.addArg("--target");
-    cmd.addArgs(Utils::transform(m_buildTargets, [rc](const QString &s) {
-            QString target = s;
-        if (isCurrentExecutableTarget(s)) {
-            if (rc) {
-                target = rc->buildKey();
-                const int pos = target.indexOf("///::///");
-                if (pos >= 0) {
-                    target = target.mid(pos + 8);
-                }
-            } else {
-                target = "<i>&lt;" + tr(ADD_RUNCONFIGURATION_TEXT) + "&gt;</i>";
-            }
+    cmd.addArgs(Utils::transform(m_buildTargets, [this](const QString &s) {
+        if (s.isEmpty()) {
+            if (RunConfiguration *rc = target()->activeRunConfiguration())
+                return rc->buildKey();
         }
-        return target;
+        return s;
     }));
 
     if (!m_cmakeArguments->value().isEmpty())
@@ -378,12 +399,6 @@ Utils::CommandLine CMakeBuildStep::cmakeCommand(RunConfiguration *rc) const
     return cmd;
 }
 
-QStringList CMakeBuildStep::knownBuildTargets()
-{
-    auto bs = qobject_cast<CMakeBuildSystem *>(buildSystem());
-    return bs ? bs->buildTargetTitles() : QStringList();
-}
-
 QString CMakeBuildStep::cleanTarget()
 {
     return QString("clean");
@@ -409,133 +424,91 @@ QStringList CMakeBuildStep::specialTargets()
     return { allTarget(), cleanTarget(), installTarget(), testTarget() };
 }
 
-//
-// CMakeBuildStepConfigWidget
-//
+QString CMakeBuildStep::activeRunConfigTarget() const
+{
+    RunConfiguration *rc = target()->activeRunConfiguration();
+    return rc ? rc->buildKey() : QString();
+}
 
-CMakeBuildStepConfigWidget::CMakeBuildStepConfigWidget(CMakeBuildStep *buildStep)
-    : BuildStepConfigWidget(buildStep)
-    , m_buildStep(buildStep)
-    , m_buildTargetsList(new QListWidget)
+BuildStepConfigWidget *CMakeBuildStep::createConfigWidget()
 {
-    setDisplayName(tr("Build", "CMakeProjectManager::CMakeBuildStepConfigWidget display name."));
+    auto widget = new BuildStepConfigWidget(this);
+
+    auto updateDetails = [this, widget] {
+        ProcessParameters param;
+        setupProcessParameters(&param);
+        param.setCommandLine(cmakeCommand());
+        widget->setSummaryText(param.summary(displayName()));
+    };
 
-    LayoutBuilder builder(this);
-    builder.addRow(buildStep->m_cmakeArguments);
-    builder.addRow(buildStep->m_toolArguments);
+    widget->setDisplayName(tr("Build", "ConfigWidget display name."));
 
-    m_buildTargetsList->setFrameStyle(QFrame::NoFrame);
-    m_buildTargetsList->setMinimumHeight(200);
+    LayoutBuilder builder(widget);
+    builder.addRow(m_cmakeArguments);
+    builder.addRow(m_toolArguments);
 
-    auto frame = new QFrame(this);
-    frame->setFrameStyle(QFrame::StyledPanel);
-    auto frameLayout = new QVBoxLayout(frame);
-    frameLayout->setContentsMargins(0, 0, 0, 0);
-    frameLayout->addWidget(Core::ItemViewFind::createSearchableWrapper(m_buildTargetsList,
-                                                                       Core::ItemViewFind::LightColored));
+    auto buildTargetsView = new QTreeView;
+    buildTargetsView->setMinimumHeight(200);
+    buildTargetsView->setModel(&m_buildTargetModel);
+    buildTargetsView->setRootIsDecorated(false);
+    buildTargetsView->setHeaderHidden(true);
+
+    auto frame = ItemViewFind::createSearchableWrapper(buildTargetsView,
+                                                       ItemViewFind::LightColored);
 
     builder.startNewRow().addItems(tr("Targets:"), frame);
 
-    buildTargetsChanged();
     updateDetails();
 
-    connect(buildStep->m_cmakeArguments, &StringAspect::changed,
-            this, &CMakeBuildStepConfigWidget::updateDetails);
-    connect(buildStep->m_toolArguments, &StringAspect::changed,
-            this, &CMakeBuildStepConfigWidget::updateDetails);
+    connect(m_cmakeArguments, &StringAspect::changed, this, updateDetails);
+    connect(m_toolArguments, &StringAspect::changed, this, updateDetails);
 
-    connect(m_buildTargetsList, &QListWidget::itemChanged,
-            this, &CMakeBuildStepConfigWidget::itemsChanged);
     connect(ProjectExplorerPlugin::instance(), &ProjectExplorerPlugin::settingsChanged,
-            this, &CMakeBuildStepConfigWidget::updateDetails);
-
-    connect(m_buildStep,
-            &CMakeBuildStep::buildTargetsChanged,
-            this,
-            &CMakeBuildStepConfigWidget::buildTargetsChanged);
-
-    connect(m_buildStep,
-            &CMakeBuildStep::targetsToBuildChanged,
-            this,
-            &CMakeBuildStepConfigWidget::updateBuildTargets);
-
-    connect(m_buildStep->buildConfiguration(),
-            &BuildConfiguration::environmentChanged,
-            this,
-            &CMakeBuildStepConfigWidget::updateDetails);
-}
+            this, updateDetails);
 
-void CMakeBuildStepConfigWidget::itemsChanged()
-{
-    const QList<QListWidgetItem *> items = [this]() {
-        QList<QListWidgetItem *> items;
-        for (int row = 0; row < m_buildTargetsList->count(); ++row)
-            items.append(m_buildTargetsList->item(row));
-        return items;
-    }();
-    const QStringList targetsToBuild = Utils::transform(Utils::filtered(items, Utils::equal(&QListWidgetItem::checkState, Qt::Checked)),
-                                                        [](const QListWidgetItem *i) { return i->data(Qt::UserRole).toString(); });
-    m_buildStep->setBuildTargets(targetsToBuild);
-    updateDetails();
+    connect(buildConfiguration(), &BuildConfiguration::environmentChanged,
+            this, updateDetails);
+
+    connect(this, &CMakeBuildStep::buildTargetsChanged, widget, updateDetails);
+
+    return widget;
 }
 
-void CMakeBuildStepConfigWidget::buildTargetsChanged()
+void CMakeBuildStep::recreateBuildTargetsModel()
 {
-    {
-        QFont italics;
-        italics.setItalic(true);
+    auto addItem = [this](const QString &target, bool special = false) {
+        auto item = new CMakeTargetItem(target, this, special);
+        m_buildTargetModel.rootItem()->appendChild(item);
+    };
 
-        auto addItem = [italics, this](const QString &buildTarget, const QString &displayName, bool special = false) {
-            auto item = new QListWidgetItem(displayName, m_buildTargetsList);
-            item->setFlags(Qt::ItemIsUserCheckable | Qt::ItemIsEnabled);
-            item->setData(Qt::UserRole, buildTarget);
-            if (special)
-                item->setFont(italics);
-        };
+    m_buildTargetModel.clear();
 
-        QSignalBlocker blocker(m_buildTargetsList);
-        m_buildTargetsList->clear();
+    auto bs = qobject_cast<CMakeBuildSystem *>(buildSystem());
+    QStringList targetList = bs ? bs->buildTargetTitles() : QStringList();
 
-        QStringList targetList = m_buildStep->knownBuildTargets();
-        targetList.sort();
+    targetList.sort();
 
-        addItem(ADD_RUNCONFIGURATION_TEXT, tr(ADD_RUNCONFIGURATION_TEXT), true);
+    addItem(QString(), true);
 
-        foreach (const QString &buildTarget, targetList)
-            addItem(buildTarget, buildTarget, CMakeBuildStep::specialTargets().contains(buildTarget));
+    for (const QString &buildTarget : qAsConst(targetList))
+        addItem(buildTarget, specialTargets().contains(buildTarget));
 
-        updateBuildTargets();
-    }
-    updateDetails();
+    updateBuildTargetsModel();
 }
 
-void CMakeBuildStepConfigWidget::updateBuildTargets()
+void CMakeBuildStep::updateBuildTargetsModel()
 {
-    const QStringList buildTargets = m_buildStep->buildTargets();
-    {
-        QSignalBlocker blocker(m_buildTargetsList);
-        for (int row = 0; row < m_buildTargetsList->count(); ++row) {
-            QListWidgetItem *item = m_buildTargetsList->item(row);
-            const QString title = item->data(Qt::UserRole).toString();
-
-            item->setCheckState(m_buildStep->buildsBuildTarget(title) ? Qt::Checked : Qt::Unchecked);
-        }
-    }
-    updateDetails();
+    emit m_buildTargetModel.layoutChanged();
+    emit buildTargetsChanged();
 }
 
-void CMakeBuildStepConfigWidget::updateDetails()
+void CMakeBuildStep::processFinished(int exitCode, QProcess::ExitStatus status)
 {
-    ProcessParameters param;
-    m_buildStep->setupProcessParameters(&param);
-    param.setCommandLine(m_buildStep->cmakeCommand(nullptr));
-
-    setSummaryText(param.summary(displayName()));
+    AbstractProcessStep::processFinished(exitCode, status);
+    emit progress(100, QString());
 }
 
-//
 // CMakeBuildStepFactory
-//
 
 CMakeBuildStepFactory::CMakeBuildStepFactory()
 {
@@ -544,12 +517,6 @@ CMakeBuildStepFactory::CMakeBuildStepFactory()
     setSupportedProjectType(Constants::CMAKE_PROJECT_ID);
 }
 
-void CMakeBuildStep::processFinished(int exitCode, QProcess::ExitStatus status)
-{
-    AbstractProcessStep::processFinished(exitCode, status);
-    emit progress(100, QString());
-}
-
 } // Internal
 } // CMakeProjectManager
 
diff --git a/src/plugins/cmakeprojectmanager/cmakebuildstep.h b/src/plugins/cmakeprojectmanager/cmakebuildstep.h
index b6a5046acfe..f2a5e7871a0 100644
--- a/src/plugins/cmakeprojectmanager/cmakebuildstep.h
+++ b/src/plugins/cmakeprojectmanager/cmakebuildstep.h
@@ -26,19 +26,33 @@
 #pragma once
 
 #include <projectexplorer/abstractprocessstep.h>
-
-#include <QRegularExpression>
+#include <utils/treemodel.h>
 
 namespace Utils { class CommandLine; }
 
-namespace ProjectExplorer {
-class RunConfiguration;
-class StringAspect;
-} // ProjectExplorer
+namespace ProjectExplorer { class StringAspect; }
 
 namespace CMakeProjectManager {
 namespace Internal {
 
+class CMakeBuildStep;
+
+class CMakeTargetItem : public Utils::TreeItem
+{
+public:
+    CMakeTargetItem() = default;
+    CMakeTargetItem(const QString &target, CMakeBuildStep *step, bool special);
+
+private:
+    QVariant data(int column, int role) const final;
+    bool setData(int column, const QVariant &data, int role) final;
+    Qt::ItemFlags flags(int column) const final;
+
+    QString m_target;
+    CMakeBuildStep *m_step = nullptr;
+    bool m_special = false;
+};
+
 class CMakeBuildStep : public ProjectExplorer::AbstractProcessStep
 {
     Q_OBJECT
@@ -47,12 +61,10 @@ public:
     CMakeBuildStep(ProjectExplorer::BuildStepList *bsl, Utils::Id id);
 
     QStringList buildTargets() const;
-    bool buildsBuildTarget(const QString &target) const;
     void setBuildTargets(const QStringList &target);
 
-    Utils::CommandLine cmakeCommand(ProjectExplorer::RunConfiguration *rc) const;
-
-    QStringList knownBuildTargets();
+    bool buildsBuildTarget(const QString &target) const;
+    void setBuildsBuildTarget(const QString &target, bool on);
 
     QVariantMap toMap() const override;
 
@@ -62,16 +74,17 @@ public:
     static QString testTarget();
     static QStringList specialTargets();
 
+    QString activeRunConfigTarget() const;
+
 signals:
-    void targetsToBuildChanged();
     void buildTargetsChanged();
 
-protected:
-    void processFinished(int exitCode, QProcess::ExitStatus status) override;
+private:
+    Utils::CommandLine cmakeCommand() const;
 
+    void processFinished(int exitCode, QProcess::ExitStatus status) override;
     bool fromMap(const QVariantMap &map) override;
 
-private:
     bool init() override;
     void setupOutputFormatter(Utils::OutputFormatter *formatter) override;
     void doRun() override;
@@ -83,14 +96,18 @@ private:
     void handleProjectWasParsed(bool success);
 
     void handleBuildTargetsChanges(bool success);
+    void recreateBuildTargetsModel();
+    void updateBuildTargetsModel();
 
     QMetaObject::Connection m_runTrigger;
 
     friend class CMakeBuildStepConfigWidget;
-    QStringList m_buildTargets;
+    QStringList m_buildTargets; // Convention: Empty string member signifies "Current executable"
     ProjectExplorer::StringAspect *m_cmakeArguments = nullptr;
     ProjectExplorer::StringAspect *m_toolArguments = nullptr;
     bool m_waiting = false;
+
+    Utils::TreeModel<Utils::TreeItem, CMakeTargetItem> m_buildTargetModel;
 };
 
 class CMakeBuildStepFactory : public ProjectExplorer::BuildStepFactory