Utils: Let aspect transition functions indicate there was a change

Helps to make it easier to reason about the necessity of emitting
*changed signals.

Change-Id: Ieab29b25f5cc2799e193417b9cab02c99501c60a
Reviewed-by: Marcus Tillmanns <marcus.tillmanns@qt.io>

src/libs/utils/aspects.cpp

@@ -1203,8 +1203,9 @@ void StringAspect::addToLayout(LayoutItem &parent)
         d->m_checker->addToLayout(parent);
 }
 
-void StringAspect::guiToBuffer()
+bool StringAspect::guiToBuffer()
 {
+    const QString old = m_buffer;
     switch (d->m_displayStyle) {
     case PathChooserDisplay:
         if (d->m_pathChooserDisplay)
@@ -1220,22 +1221,26 @@ void StringAspect::guiToBuffer()
     case LabelDisplay:
         break;
     }
+    return m_buffer != old;
 }
 
-void StringAspect::bufferToInternal()
+bool StringAspect::bufferToInternal()
 {
+    const QString old = m_internal;
     if (d->m_valueAcceptor) {
         if (const std::optional<QString> tmp = d->m_valueAcceptor(m_internal, m_buffer))
             m_internal = *tmp;
-        return;
+    } else {
+        m_internal = m_buffer;
     }
-
-    m_internal = m_buffer;
+    return m_internal != old;
 }
 
-void StringAspect::internalToBuffer()
+bool StringAspect::internalToBuffer()
 {
+    const QString old = m_buffer;
     m_buffer = d->m_displayFilter ? d->m_displayFilter(m_internal) : m_internal;
+    return m_buffer != old;
 }
 
 void StringAspect::bufferToGui()
@@ -1391,10 +1396,12 @@ void ColorAspect::addToLayout(Layouting::LayoutItem &parent)
             this, &ColorAspect::handleGuiChanged);
 }
 
-void ColorAspect::guiToBuffer()
+bool ColorAspect::guiToBuffer()
 {
+    const QColor old = m_buffer;
     if (d->m_colorButton)
         m_buffer = d->m_colorButton->color();
+    return m_buffer != old;
 }
 
 void ColorAspect::bufferToGui()
@@ -1496,12 +1503,14 @@ QAction *BoolAspect::action()
     return act;
 }
 
-void BoolAspect::guiToBuffer()
+bool BoolAspect::guiToBuffer()
 {
+    const bool old = m_buffer;
     if (d->m_button)
         m_buffer = d->m_button->isChecked();
     else if (d->m_groupBox)
         m_buffer = d->m_groupBox->isChecked();
+    return m_buffer != old;
 }
 
 void BoolAspect::bufferToGui()
@@ -1603,8 +1612,9 @@ void SelectionAspect::addToLayout(Layouting::LayoutItem &parent)
     }
 }
 
-void SelectionAspect::guiToBuffer()
+bool SelectionAspect::guiToBuffer()
 {
+    const int old = m_buffer;
     switch (d->m_displayStyle) {
     case DisplayStyle::RadioButtons:
         if (d->m_buttonGroup)
@@ -1615,6 +1625,7 @@ void SelectionAspect::guiToBuffer()
             m_buffer = d->m_comboBox->currentIndex();
         break;
     }
+    return m_buffer != old;
 }
 
 void SelectionAspect::bufferToGui()
@@ -1806,8 +1817,9 @@ void MultiSelectionAspect::bufferToGui()
     }
 }
 
-void MultiSelectionAspect::guiToBuffer()
+bool MultiSelectionAspect::guiToBuffer()
 {
+    const QStringList old = m_buffer;
     if (d->m_listView) {
         m_buffer.clear();
         const int n = d->m_listView->count();
@@ -1818,6 +1830,7 @@ void MultiSelectionAspect::guiToBuffer()
                 m_buffer.append(item->text());
         }
     }
+    return m_buffer != old;
 }
 
 
@@ -1869,10 +1882,12 @@ void IntegerAspect::addToLayout(Layouting::LayoutItem &parent)
             this, &IntegerAspect::handleGuiChanged);
 }
 
-void IntegerAspect::guiToBuffer()
+bool IntegerAspect::guiToBuffer()
 {
+    const qint64 old = m_buffer;
     if (d->m_spinBox)
         m_buffer = d->m_spinBox->value() * d->m_displayScaleFactor;
+    return m_buffer != old;
 }
 
 void IntegerAspect::bufferToGui()
@@ -1968,10 +1983,12 @@ void DoubleAspect::addToLayout(LayoutItem &builder)
             this, &DoubleAspect::handleGuiChanged);
 }
 
-void DoubleAspect::guiToBuffer()
+bool DoubleAspect::guiToBuffer()
 {
+    const double old = m_buffer;
     if (d->m_spinBox)
         m_buffer = d->m_spinBox->value();
+    return m_buffer != old;
 }
 
 void DoubleAspect::bufferToGui()
@@ -2447,46 +2464,61 @@ BaseAspect::Data::Ptr BaseAspect::extractData() const
     No-op otherwise.
 */
 void BaseAspect::bufferToGui()
-{}
+{
+}
 
 /*
     Mirrors the data stored in GUI element if they are already created to
     the internal volatile value.
 
     No-op otherwise.
+
+    \return true when the buffered volatile value changed.
 */
-void BaseAspect::guiToBuffer()
-{}
+bool BaseAspect::guiToBuffer()
+{
+    return false;
+}
 
 /*
     Mirrors buffered volatile value to the internal value.
     This function is used for \c apply().
+
+    \return true when the internal value changed.
 */
 
-void BaseAspect::bufferToInternal()
-{}
+bool BaseAspect::bufferToInternal()
+{
+    return false;
+}
 
 
-void BaseAspect::internalToBuffer()
-{}
+bool BaseAspect::internalToBuffer()
+{
+    return false;
+}
 
 /*
     Applies common postprocessing like macro expansion.
 */
 
-void BaseAspect::internalToExternal()
-{}
+bool BaseAspect::internalToExternal()
+{
+    return false;
+}
 
 /*
     Mirrors externally visible value to internal volatile value.
 */
-void BaseAspect::externalToInternal()
-{}
+bool BaseAspect::externalToInternal()
+{
+    return false;
+}
 
 void BaseAspect::handleGuiChanged()
 {
-    guiToBuffer();
-    emit volatileValueChanged();
+    if (guiToBuffer())
+        volatileValueChanged();
     if (isAutoApply())
         apply();
 }

src/libs/utils/aspects.h

@@ -165,17 +165,18 @@ public:
     static QSettings *settings();
 
 signals:
-    void changed();
+    void changed(); // "internal"
     void volatileValueChanged();
+    void externalValueChanged();
     void labelLinkActivated(const QString &link);
 
 protected:
-    virtual void internalToBuffer();
-    virtual void bufferToInternal();
+    virtual bool internalToBuffer();
+    virtual bool bufferToInternal();
     virtual void bufferToGui();
-    virtual void guiToBuffer();
-    virtual void internalToExternal();
-    virtual void externalToInternal();
+    virtual bool guiToBuffer();
+    virtual bool internalToExternal();
+    virtual bool externalToInternal();
 
     virtual void handleGuiChanged();
 
@@ -247,10 +248,14 @@ public:
 
     void setValue(const ValueType &value)
     {
+        const bool changes = m_internal != value;
         m_internal = value;
-        internalToBuffer();
+        if (internalToBuffer())
+            emit volatileValueChanged();
         bufferToGui();
         internalToExternal();
+        if (changes)
+            emit changed();
     }
 
     void setValueQuietly(const ValueType &value)
@@ -281,24 +286,36 @@ protected:
         return m_internal != m_buffer;
     }
 
-    void internalToBuffer() override
+    bool internalToBuffer() override
     {
+        if (m_buffer == m_internal)
+            return false;
         m_buffer = m_internal;
+        return true;
     }
 
-    void bufferToInternal() override
+    bool bufferToInternal() override
     {
+        if (m_buffer == m_internal)
+            return false;
         m_internal = m_buffer;
+        return true;
     }
 
-    void internalToExternal() override
+    bool internalToExternal() override
     {
+        if (m_external == m_internal)
+            return false;
         m_external = m_internal;
+        return true;
     }
 
-    void externalToInternal() override
+    bool externalToInternal() override
     {
+        if (m_external == m_internal)
+            return false;
         m_internal = m_external;
+        return true;
     }
 
     QVariant variantValue() const override
@@ -357,7 +374,7 @@ public:
 
 private:
     void bufferToGui() override;
-    void guiToBuffer() override;
+    bool guiToBuffer() override;
 
     std::unique_ptr<Internal::BoolAspectPrivate> d;
 };
@@ -374,7 +391,7 @@ public:
 
 private:
     void bufferToGui() override;
-    void guiToBuffer() override;
+    bool guiToBuffer() override;
 
     std::unique_ptr<Internal::ColorAspectPrivate> d;
 };
@@ -422,7 +439,7 @@ public:
 
 protected:
     void bufferToGui() override;
-    void guiToBuffer() override;
+    bool guiToBuffer() override;
 
 private:
     std::unique_ptr<Internal::SelectionAspectPrivate> d;
@@ -446,7 +463,7 @@ public:
 
 protected:
     void bufferToGui() override;
-    void guiToBuffer() override;
+    bool guiToBuffer() override;
 
 private:
     std::unique_ptr<Internal::MultiSelectionAspectPrivate> d;
@@ -522,10 +539,10 @@ signals:
 
 protected:
     void bufferToGui() override;
-    void guiToBuffer() override;
+    bool guiToBuffer() override;
 
-    void internalToBuffer() override;
-    void bufferToInternal() override;
+    bool internalToBuffer() override;
+    bool bufferToInternal() override;
 
     std::unique_ptr<Internal::StringAspectPrivate> d;
 };
@@ -573,7 +590,7 @@ public:
 
 protected:
     void bufferToGui() override;
-    void guiToBuffer() override;
+    bool guiToBuffer() override;
 
 private:
     std::unique_ptr<Internal::IntegerAspectPrivate> d;
@@ -597,7 +614,7 @@ public:
 
 protected:
     void bufferToGui() override;
-    void guiToBuffer() override;
+    bool guiToBuffer() override;
 
 private:
     std::unique_ptr<Internal::DoubleAspectPrivate> d;

src/plugins/debugger/commonoptionspage.h

@@ -31,7 +31,7 @@ public:
     void writeSettings() const override;
 
 private:
-    void guiToBuffer() override;
+    bool guiToBuffer() override;
     void bufferToGui() override;
 
     SourcePathMapAspectPrivate *d = nullptr;

src/plugins/debugger/debuggersourcepathmappingwidget.cpp

@@ -470,10 +470,12 @@ void SourcePathMapAspect::addToLayout(Layouting::LayoutItem &parent)
     parent.addItem(d->m_widget.data());
 }
 
-void SourcePathMapAspect::guiToBuffer()
+bool SourcePathMapAspect::guiToBuffer()
 {
+    const SourcePathMap old = m_buffer;
     if (d->m_widget)
         m_buffer = d->m_widget->sourcePathMap();
+    return m_buffer != old;
 }
 
 void SourcePathMapAspect::bufferToGui()

src/plugins/valgrind/valgrindsettings.cpp

@@ -151,7 +151,7 @@ void SuppressionAspect::addToLayout(Layouting::LayoutItem &parent)
 
 void SuppressionAspect::fromMap(const QVariantMap &map)
 {
-    BaseAspect::fromMap(map);
+    BaseAspect::fromMap(map); // FIXME Looks wrong, as it skips the intermediate level
 }
 
 void SuppressionAspect::toMap(QVariantMap &map) const
@@ -159,11 +159,13 @@ void SuppressionAspect::toMap(QVariantMap &map) const
     BaseAspect::toMap(map);
 }
 
-void SuppressionAspect::guiToBuffer()
+bool SuppressionAspect::guiToBuffer()
 {
+    const FilePaths old = m_buffer;
     m_buffer.clear();
     for (int i = 0; i < d->m_model.rowCount(); ++i)
         m_buffer.append(FilePath::fromUserInput(d->m_model.item(i)->text()));
+    return m_buffer != old;
 }
 
 void SuppressionAspect::bufferToGui()

src/plugins/valgrind/valgrindsettings.h

@@ -29,7 +29,7 @@ public:
 
 private:
     void bufferToGui() override;
-    void guiToBuffer() override;
+    bool guiToBuffer() override;
 
     friend class ValgrindBaseSettings;
     SuppressionAspectPrivate *d = nullptr;