From 632582371c972811cf3439743c8a6b56d2ff0c0a Mon Sep 17 00:00:00 2001
From: hjk <hjk@qt.io>
Date: Fri, 9 Oct 2020 13:12:46 +0200
Subject: 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     |  3 ---
 src/plugins/projectexplorer/buildstep.cpp      | 31 +++++++++++++++++---------
 src/plugins/projectexplorer/buildstep.h        |  5 +++--
 src/plugins/projectexplorer/buildstepspage.cpp |  2 +-
 src/plugins/projectexplorer/makestep.cpp       |  2 --
 5 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/src/plugins/nim/project/nimbletaskstep.cpp b/src/plugins/nim/project/nimbletaskstep.cpp
index 1cb0505089..f41d53da90 100644
--- a/src/plugins/nim/project/nimbletaskstep.cpp
+++ b/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());
diff --git a/src/plugins/projectexplorer/buildstep.cpp b/src/plugins/projectexplorer/buildstep.cpp
index d5aaf6ea78..6e20887d70 100644
--- a/src/plugins/projectexplorer/buildstep.cpp
+++ b/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
diff --git a/src/plugins/projectexplorer/buildstep.h b/src/plugins/projectexplorer/buildstep.h
index 6fda3e7b03..78d5331034 100644
--- a/src/plugins/projectexplorer/buildstep.h
+++ b/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;
diff --git a/src/plugins/projectexplorer/buildstepspage.cpp b/src/plugins/projectexplorer/buildstepspage.cpp
index cc642d0d93..f23434d6ac 100644
--- a/src/plugins/projectexplorer/buildstepspage.cpp
+++ b/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;
diff --git a/src/plugins/projectexplorer/makestep.cpp b/src/plugins/projectexplorer/makestep.cpp
index 86c1fe74a8..c411ac34c9 100644
--- a/src/plugins/projectexplorer/makestep.cpp
+++ b/src/plugins/projectexplorer/makestep.cpp
@@ -416,8 +416,6 @@ QWidget *MakeStep::createConfigWidget()
         m_nonOverrideWarning->setVisible(makeflagsJobCountMismatch()
                                          && !jobCountOverridesMakeflags());
         disableInSubDirsCheckBox->setChecked(!m_enabledForSubDirs);
-
-        recreateSummary();
     };
 
     updateDetails();
-- 
cgit v1.2.3