diff --git a/src/libs/utils/aspects.cpp b/src/libs/utils/aspects.cpp index 2ec4bfe280f..2fc74b144b4 100644 --- a/src/libs/utils/aspects.cpp +++ b/src/libs/utils/aspects.cpp @@ -47,9 +47,12 @@ QSettings *BaseAspect::settings() return theSettings; } -namespace Internal { +BaseAspect::Changes::Changes() +{ + memset(this, 0, sizeof(*this)); +} -class BaseAspectPrivate +class Internal::BaseAspectPrivate { public: explicit BaseAspectPrivate(AspectContainer *container) : m_container(container) {} @@ -82,8 +85,6 @@ public: QList m_dataExtractors; }; -} // Internal - /*! \class Utils::BaseAspect \inmodule QtCreator @@ -144,9 +145,10 @@ QVariant BaseAspect::variantValue() const Prefer the typed setValue() of derived classes. */ -void BaseAspect::setVariantValue(const QVariant &value) +void BaseAspect::setVariantValue(const QVariant &value, Announcement howToAnnounce) { Q_UNUSED(value) + Q_UNUSED(howToAnnounce) QTC_CHECK(false); } @@ -156,19 +158,6 @@ void BaseAspect::setDefaultVariantValue(const QVariant &value) QTC_CHECK(false); } -/*! - \internal - - Sets \a value without emitting signals. - - Prefer the typed setValueQuietly() of derived classes. -*/ -void BaseAspect::setVariantValueQuietly(const QVariant &value) -{ - Q_UNUSED(value) - QTC_CHECK(false); -} - QVariant BaseAspect::defaultVariantValue() const { return {}; @@ -488,26 +477,18 @@ void createItem(Layouting::LayoutItem *item, const BaseAspect *aspect) */ void BaseAspect::apply() { - if (isDirty()) { - silentApply(); - if (hasAction()) - emit action()->triggered(variantValue().toBool()); - emit changed(); - } + // We assume m_buffer to reflect current gui state as invariant after signalling settled down. + // It's an aspect (-subclass) implementation problem if this doesn't hold. Fix it up and bark. + QTC_CHECK(!guiToBuffer()); + + if (!bufferToInternal()) // Nothing to do. + return; + + Changes changes; + changes.internalFromBuffer = true; + announceChanges(changes); } -/*! - Updates this aspect's value from user-initiated changes in the widget. - - \returns whether the value changed. Does not emit signals. -*/ - -void BaseAspect::silentApply() -{ - guiToBuffer(); - bufferToInternal(); - internalToExternal(); -} /*! Discard user changes in the widget and restore widget contents from aspect's value. @@ -532,6 +513,21 @@ bool BaseAspect::hasAction() const return d->m_action != nullptr; } +void BaseAspect::announceChanges(Changes changes, Announcement howToAnnounce) +{ + if (howToAnnounce == BeQuiet) + return; + + if (changes.bufferFromInternal || changes.bufferFromOutside || changes.bufferFromGui) + emit volatileValueChanged(); + + if (changes.internalFromOutside || changes.internalFromBuffer) { + emit changed(); + if (hasAction()) + emit action()->triggered(variantValue().toBool()); + } +} + bool BaseAspect::isDirty() { return false; @@ -578,7 +574,7 @@ void BaseAspect::fromMap(const QVariantMap &map) if (settingsKey().isEmpty()) return; const QVariant val = map.value(settingsKey(), toSettingsValue(defaultVariantValue())); - setVariantValue(fromSettingsValue(val)); + setVariantValue(fromSettingsValue(val), BeQuiet); } /*! @@ -596,7 +592,7 @@ void BaseAspect::readSettings() if (settingsKey().isEmpty()) return; const QVariant val = settings()->value(settingsKey()); - setVariantValue(val.isValid() ? fromSettingsValue(val) : defaultVariantValue()); + setVariantValue(val.isValid() ? fromSettingsValue(val) : defaultVariantValue(), BeQuiet); } void BaseAspect::writeSettings() const @@ -908,7 +904,7 @@ void StringAspect::setValueAcceptor(StringAspect::ValueAcceptor &&acceptor) void StringAspect::fromMap(const QVariantMap &map) { if (!settingsKey().isEmpty()) - setValueQuietly(map.value(settingsKey(), defaultValue()).toString()); + setValue(map.value(settingsKey(), defaultValue()).toString(), BeQuiet); d->m_checkerImpl.fromMap(map); } @@ -1112,6 +1108,12 @@ void StringAspect::addToLayout(LayoutItem &parent) d->m_checkerImpl.addToLayoutLast(parent); } +QString StringAspect::expandedValue() const +{ + // FIXME: Use macroexpander here later. + return m_internal; +} + bool StringAspect::guiToBuffer() { if (d->m_lineEditDisplay) @@ -1266,9 +1268,14 @@ QString FilePathAspect::value() const a file path. */ -void FilePathAspect::setValue(const FilePath &filePath) +void FilePathAspect::setValue(const FilePath &filePath, Announcement howToAnnounce) { - TypedAspect::setValue(filePath.toUserOutput()); + TypedAspect::setValue(filePath.toUserOutput(), howToAnnounce); +} + +void FilePathAspect::setValue(const QString &filePath, Announcement howToAnnounce) +{ + TypedAspect::setValue(filePath, howToAnnounce); } void FilePathAspect::setDefaultValue(const FilePath &filePath) @@ -1402,7 +1409,7 @@ void FilePathAspect::addToLayout(Layouting::LayoutItem &parent) void FilePathAspect::fromMap(const QVariantMap &map) { if (!settingsKey().isEmpty()) - setValueQuietly(map.value(settingsKey(), defaultValue()).toString()); + setValue(map.value(settingsKey(), defaultValue()).toString(), BeQuiet); d->m_checkerImpl.fromMap(map); } @@ -2545,7 +2552,7 @@ void AspectContainer::finish() void AspectContainer::reset() { for (BaseAspect *aspect : std::as_const(d->m_items)) - aspect->setVariantValueQuietly(aspect->defaultVariantValue()); + aspect->setVariantValue(aspect->defaultVariantValue()); } void AspectContainer::setAutoApply(bool on) @@ -2643,23 +2650,6 @@ bool BaseAspect::internalToBuffer() return false; } -/* - Applies common postprocessing like macro expansion. -*/ - -bool BaseAspect::internalToExternal() -{ - return false; -} - -/* - Mirrors externally visible value to internal volatile value. -*/ -bool BaseAspect::externalToInternal() -{ - return false; -} - void BaseAspect::handleGuiChanged() { if (guiToBuffer()) diff --git a/src/libs/utils/aspects.h b/src/libs/utils/aspects.h index 14bb3ed6f73..b4b2d4099a9 100644 --- a/src/libs/utils/aspects.h +++ b/src/libs/utils/aspects.h @@ -53,9 +53,10 @@ public: Id id() const; void setId(Id id); + enum Announcement { DoEmit, BeQuiet }; + virtual QVariant variantValue() const; - virtual void setVariantValue(const QVariant &value); - virtual void setVariantValueQuietly(const QVariant &value); + virtual void setVariantValue(const QVariant &value, Announcement = DoEmit); virtual QVariant defaultVariantValue() const; virtual void setDefaultVariantValue(const QVariant &value); @@ -114,12 +115,24 @@ public: QVariant fromSettingsValue(const QVariant &val) const; virtual void apply(); - virtual void silentApply(); virtual void cancel(); virtual void finish(); virtual bool isDirty(); bool hasAction() const; + struct QTCREATOR_UTILS_EXPORT Changes + { + Changes(); + + unsigned internalFromOutside : 1; + unsigned internalFromBuffer : 1; + unsigned bufferFromOutside : 1; + unsigned bufferFromInternal : 1; + unsigned bufferFromGui : 1; + }; + + void announceChanges(Changes changes, Announcement howToAnnounce = DoEmit); + class QTCREATOR_UTILS_EXPORT Data { public: @@ -172,7 +185,6 @@ public: signals: void changed(); // "internal" void volatileValueChanged(); - void externalValueChanged(); void labelLinkActivated(const QString &link); void checkedChanged(); @@ -181,8 +193,6 @@ protected: virtual bool bufferToInternal(); virtual void bufferToGui(); virtual bool guiToBuffer(); - virtual bool internalToExternal(); - virtual bool externalToInternal(); virtual void handleGuiChanged(); @@ -253,48 +263,41 @@ public: ValueType value; }; - ValueType operator()() const { return m_external; } - ValueType value() const { return m_external; } + ValueType operator()() const { return m_internal; } + ValueType value() const { return m_internal; } ValueType defaultValue() const { return m_default; } + ValueType volatileValue() const { return m_buffer; } + // We assume that this is only used in the ctor and no signalling is needed. + // If it is used elsewhere changes have to be detected and signalled externally. void setDefaultValue(const ValueType &value) { m_default = value; - setValue(value); - } - - void setValue(const ValueType &value) - { - const bool changes = m_internal != value; m_internal = value; - if (internalToBuffer()) - emit volatileValueChanged(); - bufferToGui(); - internalToExternal(); - if (changes) - emit changed(); + internalToBuffer(); // Might be more than a plain copy. } - void setValueQuietly(const ValueType &value) + void setValue(const ValueType &value, Announcement howToAnnounce = DoEmit) { - m_internal = value; - internalToBuffer(); - bufferToGui(); - internalToExternal(); + Changes changes; + changes.internalFromOutside = updateStorage(m_internal, value); + if (internalToBuffer()) { + changes.bufferFromInternal = true; + bufferToGui(); + } + announceChanges(changes, howToAnnounce); } - ValueType volatileValue() const + void setVolatileValue(const ValueType &value, Announcement howToAnnounce = DoEmit) { - return m_buffer; - } - - void setVolatileValue(const ValueType &value) - { - if (m_buffer == value) - return; - m_buffer = value; - bufferToGui(); - emit volatileValueChanged(); + Changes changes; + if (updateStorage(m_buffer, value)) { + changes.bufferFromOutside = true; + bufferToGui(); + } + if (isAutoApply() && bufferToInternal()) + changes.internalFromBuffer = true; + announceChanges(changes, howToAnnounce); } protected: @@ -313,29 +316,14 @@ protected: return updateStorage(m_internal, m_buffer); } - bool internalToExternal() override - { - return updateStorage(m_external, m_internal); - } - - bool externalToInternal() override - { - return updateStorage(m_internal, m_external); - } - QVariant variantValue() const override { - return QVariant::fromValue(m_external); + return QVariant::fromValue(m_internal); } - void setVariantValue(const QVariant &value) override + void setVariantValue(const QVariant &value, Announcement howToAnnounce = DoEmit) override { - setValue(value.value()); - } - - void setVariantValueQuietly(const QVariant &value) override - { - setValueQuietly(value.value()); + setValue(value.value(), howToAnnounce); } QVariant defaultVariantValue() const override @@ -345,11 +333,10 @@ protected: void setDefaultVariantValue(const QVariant &value) override { - m_default = value.value(); + setDefaultValue(value.value()); } ValueType m_default{}; - ValueType m_external{}; ValueType m_internal{}; ValueType m_buffer{}; }; @@ -366,7 +353,6 @@ public: void setInternalToBuffer(const Updater &updater) { m_internalToBuffer = updater; } void setBufferToInternal(const Updater &updater) { m_bufferToInternal = updater; } void setInternalToExternal(const Updater &updater) { m_internalToExternal = updater; } - void setExternalToInternal(const Updater &updater) { m_externalToInternal = updater; } protected: bool internalToBuffer() override @@ -383,24 +369,18 @@ protected: return Base::bufferToInternal(); } - bool internalToExternal() override + ValueType expandedValue() { - if (m_internalToExternal) - return m_internalToExternal(Base::m_external, Base::m_internal); - return Base::internalToExternal(); - } - - bool externalToInternal() override - { - if (m_externalToInternal) - return m_externalToInternal(Base::m_internal, Base::m_external); - return Base::externalToInternal(); + if (!m_internalToExternal) + return Base::m_internal; + ValueType val; + m_internalToExternal(val, Base::m_internal); + return val; } Updater m_internalToBuffer; Updater m_bufferToInternal; Updater m_internalToExternal; - Updater m_externalToInternal; }; class QTCREATOR_UTILS_EXPORT BoolAspect : public TypedAspect @@ -536,6 +516,9 @@ public: void addToLayout(Layouting::LayoutItem &parent) override; + QString operator()() const { return expandedValue(); } + QString expandedValue() const; + // Hook between UI and StringAspect: using ValueAcceptor = std::function(const QString &, const QString &)>; void setValueAcceptor(ValueAcceptor &&acceptor); @@ -599,7 +582,8 @@ public: FilePath operator()() const; FilePath expandedValue() const; QString value() const; - void setValue(const FilePath &filePath); + void setValue(const FilePath &filePath, Announcement howToAnnounce = DoEmit); + void setValue(const QString &filePath, Announcement howToAnnounce = DoEmit); void setDefaultValue(const FilePath &filePath); void setPromptDialogFilter(const QString &filter); diff --git a/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp b/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp index d2653baefeb..746c78f5491 100644 --- a/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp +++ b/src/plugins/cmakeprojectmanager/cmakebuildconfiguration.cpp @@ -2071,10 +2071,7 @@ QString CMakeBuildSystem::cmakeBuildType() const void CMakeBuildConfiguration::setCMakeBuildType(const QString &cmakeBuildType, bool quiet) { - if (quiet) - buildTypeAspect.setValueQuietly(cmakeBuildType); - else - buildTypeAspect.setValue(cmakeBuildType); + buildTypeAspect.setValue(cmakeBuildType, quiet ? BaseAspect::BeQuiet : BaseAspect::DoEmit); } namespace Internal { @@ -2134,7 +2131,7 @@ void InitialCMakeArgumentsAspect::setAllValues(const QString &values, QStringLis // Display the unknown arguments in "Additional CMake Options" const QString additionalOptionsValue = ProcessArgs::joinArgs(additionalOptions); - setValueQuietly(additionalOptionsValue); + setValue(additionalOptionsValue, BeQuiet); } void InitialCMakeArgumentsAspect::setCMakeConfiguration(const CMakeConfig &config) diff --git a/src/plugins/debugger/registerpostmortemaction.cpp b/src/plugins/debugger/registerpostmortemaction.cpp index a7f68083180..4cfbdc02e94 100644 --- a/src/plugins/debugger/registerpostmortemaction.cpp +++ b/src/plugins/debugger/registerpostmortemaction.cpp @@ -60,7 +60,7 @@ void RegisterPostMortemAction::readSettings() registered = isRegistered(handle, debuggerCall(), &errorMessage); if (handle) RegCloseKey(handle); - setValueQuietly(registered); + setValue(registered, BaseAspect::BeQuiet); } } // namespace Internal diff --git a/src/plugins/projectexplorer/processstep.cpp b/src/plugins/projectexplorer/processstep.cpp index e2aa69dff9b..782eadf91df 100644 --- a/src/plugins/projectexplorer/processstep.cpp +++ b/src/plugins/projectexplorer/processstep.cpp @@ -37,7 +37,7 @@ public: arguments.setLabelText(Tr::tr("Arguments:")); workingDirectory.setSettingsKey(PROCESS_WORKINGDIRECTORY_KEY); - workingDirectory.setValue(Constants::DEFAULT_WORKING_DIR); + workingDirectory.setValue(QString(Constants::DEFAULT_WORKING_DIR)); workingDirectory.setLabelText(Tr::tr("Working directory:")); workingDirectory.setExpectedKind(PathChooser::Directory);