From 7cc607875667debf14d2fcd46a0c051bdb1027bb Mon Sep 17 00:00:00 2001 From: hjk Date: Thu, 20 Jul 2023 14:25:13 +0200 Subject: [PATCH] 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 --- src/libs/utils/aspects.cpp | 82 +++++++++++++------ src/libs/utils/aspects.h | 57 ++++++++----- src/plugins/debugger/commonoptionspage.h | 2 +- .../debuggersourcepathmappingwidget.cpp | 4 +- src/plugins/valgrind/valgrindsettings.cpp | 6 +- src/plugins/valgrind/valgrindsettings.h | 2 +- 6 files changed, 103 insertions(+), 50 deletions(-) diff --git a/src/libs/utils/aspects.cpp b/src/libs/utils/aspects.cpp index 55a9ee77fa7..381c396b6d0 100644 --- a/src/libs/utils/aspects.cpp +++ b/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 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(); } diff --git a/src/libs/utils/aspects.h b/src/libs/utils/aspects.h index ea12b9b7b82..7a85f2af649 100644 --- a/src/libs/utils/aspects.h +++ b/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 d; }; @@ -374,7 +391,7 @@ public: private: void bufferToGui() override; - void guiToBuffer() override; + bool guiToBuffer() override; std::unique_ptr d; }; @@ -422,7 +439,7 @@ public: protected: void bufferToGui() override; - void guiToBuffer() override; + bool guiToBuffer() override; private: std::unique_ptr d; @@ -446,7 +463,7 @@ public: protected: void bufferToGui() override; - void guiToBuffer() override; + bool guiToBuffer() override; private: std::unique_ptr 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 d; }; @@ -573,7 +590,7 @@ public: protected: void bufferToGui() override; - void guiToBuffer() override; + bool guiToBuffer() override; private: std::unique_ptr d; @@ -597,7 +614,7 @@ public: protected: void bufferToGui() override; - void guiToBuffer() override; + bool guiToBuffer() override; private: std::unique_ptr d; diff --git a/src/plugins/debugger/commonoptionspage.h b/src/plugins/debugger/commonoptionspage.h index f5806b39d69..f5624191bfb 100644 --- a/src/plugins/debugger/commonoptionspage.h +++ b/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; diff --git a/src/plugins/debugger/debuggersourcepathmappingwidget.cpp b/src/plugins/debugger/debuggersourcepathmappingwidget.cpp index 6ec92416ed8..8e4d2bef147 100644 --- a/src/plugins/debugger/debuggersourcepathmappingwidget.cpp +++ b/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() diff --git a/src/plugins/valgrind/valgrindsettings.cpp b/src/plugins/valgrind/valgrindsettings.cpp index 8331c5331c8..198312f2f5c 100644 --- a/src/plugins/valgrind/valgrindsettings.cpp +++ b/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() diff --git a/src/plugins/valgrind/valgrindsettings.h b/src/plugins/valgrind/valgrindsettings.h index aac01dac46d..5f68e93f2c2 100644 --- a/src/plugins/valgrind/valgrindsettings.h +++ b/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;