ProjectExplorer: Restructure BuildStep summary update

Move the first update from immediately after setting the updater (not
necessary when the summary never got shown, and too early to take
fromMap() data into account) to widget creation time.

This fixes the "Invalid Command" display for CustomBuildSteps
after project loading even if the command is valid.

Alos make sure we don't accumulate connections on repeated
config widget creations.

Also make sure this is not lost in createConfigWidget
re-implementations.

Change-Id: Ib8d07fbf1f0aefc45c66f74617c8fc882dc1f68e
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

src/plugins/nim/project/nimbletaskstep.cpp

@@ -124,9 +124,6 @@ QWidget *NimbleTaskStep::createConfigWidget()
 
     connect(buildSystem, &NimbleBuildSystem::tasksChanged, this, &NimbleTaskStep::updateTaskList);
 
-    connect(m_taskName, &StringAspect::changed, this, &BuildStep::recreateSummary);
-    connect(m_taskArgs, &StringAspect::changed, this, &BuildStep::recreateSummary);
-
     setSummaryUpdater([this] {
         return QString("<b>%1:</b> nimble %2 %3")
                 .arg(displayName(), m_taskName->value(), m_taskArgs->value());

src/plugins/projectexplorer/buildstep.cpp

@@ -157,6 +157,26 @@ void BuildStep::cancel()
     doCancel();
 }
 
+QWidget *BuildStep::doCreateConfigWidget()
+{
+    QWidget *widget = createConfigWidget();
+
+    const auto recreateSummary = [this] {
+        if (m_summaryUpdater)
+            setSummaryText(m_summaryUpdater());
+    };
+
+    for (BaseAspect *aspect : qAsConst(m_aspects))
+        connect(aspect, &BaseAspect::changed, widget, recreateSummary);
+
+    connect(buildConfiguration(), &BuildConfiguration::buildDirectoryChanged,
+            widget, recreateSummary);
+
+    recreateSummary();
+
+    return widget;
+}
+
 QWidget *BuildStep::createConfigWidget()
 {
     auto widget = new QWidget;
@@ -165,12 +185,8 @@ QWidget *BuildStep::createConfigWidget()
     for (BaseAspect *aspect : qAsConst(m_aspects)) {
         if (aspect->isVisible())
             aspect->addToLayout(builder.finishRow());
-        connect(aspect, &BaseAspect::changed, this, &BuildStep::recreateSummary);
     }
 
-    connect(buildConfiguration(), &BuildConfiguration::buildDirectoryChanged,
-            this, &BuildStep::recreateSummary);
-
     if (m_addMacroExpander)
         VariableChooser::addSupportForChildWidgets(widget, macroExpander());
 
@@ -500,13 +516,6 @@ void BuildStep::setSummaryText(const QString &summaryText)
 void BuildStep::setSummaryUpdater(const std::function<QString()> &summaryUpdater)
 {
     m_summaryUpdater = summaryUpdater;
-    recreateSummary();
-}
-
-void BuildStep::recreateSummary()
-{
-    if (m_summaryUpdater)
-        setSummaryText(m_summaryUpdater());
 }
 
 } // ProjectExplorer

src/plugins/projectexplorer/buildstep.h

@@ -70,7 +70,6 @@ public:
     virtual bool init() = 0;
     void run();
     void cancel();
-    virtual QWidget *createConfigWidget();
 
     bool fromMap(const QVariantMap &map) override;
     QVariantMap toMap() const override;
@@ -120,7 +119,7 @@ public:
     QString summaryText() const;
     void setSummaryText(const QString &summaryText);
 
-    void recreateSummary();
+    QWidget *doCreateConfigWidget();
 
 signals:
     void updateSummary();
@@ -141,6 +140,8 @@ signals:
     void finished(bool result);
 
 protected:
+    virtual QWidget *createConfigWidget();
+
     void runInThread(const std::function<bool()> &syncImpl);
 
     std::function<bool()> cancelChecker() const;

src/plugins/projectexplorer/buildstepspage.cpp

@@ -170,7 +170,7 @@ void ToolWidget::setDownVisible(bool b)
 BuildStepsWidgetData::BuildStepsWidgetData(BuildStep *s) :
     step(s), widget(nullptr), detailsWidget(nullptr)
 {
-    widget = s->createConfigWidget();
+    widget = s->doCreateConfigWidget();
     Q_ASSERT(widget);
 
     detailsWidget = new DetailsWidget;

src/plugins/projectexplorer/makestep.cpp

@@ -416,8 +416,6 @@ QWidget *MakeStep::createConfigWidget()
         m_nonOverrideWarning->setVisible(makeflagsJobCountMismatch()
                                          && !jobCountOverridesMakeflags());
         disableInSubDirsCheckBox->setChecked(!m_enabledForSubDirs);
-
-        recreateSummary();
     };
 
     updateDetails();