From 2bf376cffb1d80f0c92b8b4941b6b2deca509619 Mon Sep 17 00:00:00 2001 From: Pieter Hijma Date: Fri, 13 Jun 2025 15:32:10 +0200 Subject: [PATCH] Core: Add undo/redo support to property renaming --- src/App/Document.cpp | 23 ++++++++++++++--- src/App/Document.h | 5 ++++ src/App/DocumentObject.cpp | 10 ++++++++ src/App/DocumentObject.h | 2 ++ src/App/PropertyContainer.h | 2 +- src/App/Transactions.cpp | 51 ++++++++++++++++++++++++++++++++++--- src/App/Transactions.h | 8 ++++++ 7 files changed, 94 insertions(+), 7 deletions(-) diff --git a/src/App/Document.cpp b/src/App/Document.cpp index 57458dd9fd..4c813d5567 100644 --- a/src/App/Document.cpp +++ b/src/App/Document.cpp @@ -262,8 +262,9 @@ bool Document::redo(const int id) return false; } -void Document::addOrRemovePropertyOfObject(TransactionalObject* obj, - const Property* prop, const bool add) +void Document::changePropertyOfObject(TransactionalObject* obj, + const Property* prop, + const std::function& changeFunc) { if (!prop || !obj || !obj->isAttachedToDocument()) { return; @@ -278,10 +279,26 @@ void Document::addOrRemovePropertyOfObject(TransactionalObject* obj, } } if (d->activeUndoTransaction && !d->rollback) { - d->activeUndoTransaction->addOrRemoveProperty(obj, prop, add); + changeFunc(); } } +void Document::renamePropertyOfObject(TransactionalObject* obj, + const Property* prop, const char* oldName) +{ + changePropertyOfObject(obj, prop, [this, obj, prop, oldName]() { + d->activeUndoTransaction->renameProperty(obj, prop, oldName); + }); +} + +void Document::addOrRemovePropertyOfObject(TransactionalObject* obj, + const Property* prop, const bool add) +{ + changePropertyOfObject(obj, prop, [this, obj, prop, add]() { + d->activeUndoTransaction->addOrRemoveProperty(obj, prop, add); + }); +} + bool Document::isPerformingTransaction() const { return d->undoing || d->rollback; diff --git a/src/App/Document.h b/src/App/Document.h index 53308379c1..e15978cd1c 100644 --- a/src/App/Document.h +++ b/src/App/Document.h @@ -518,6 +518,7 @@ public: bool isPerformingTransaction() const; /// \internal add or remove property from a transactional object void addOrRemovePropertyOfObject(TransactionalObject*, const Property* prop, bool add); + void renamePropertyOfObject(TransactionalObject*, const Property* prop, const char* newName); //@} /** @name dependency stuff */ @@ -699,6 +700,10 @@ protected: /// Internally called by Application to abort the running transaction. void _abortTransaction(); +private: + void changePropertyOfObject(TransactionalObject* obj, const Property* prop, + const std::function& changeFunc); + private: // # Data Member of the document // +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ diff --git a/src/App/DocumentObject.cpp b/src/App/DocumentObject.cpp index daa1e4fe6c..b21509cbe0 100644 --- a/src/App/DocumentObject.cpp +++ b/src/App/DocumentObject.cpp @@ -712,6 +712,16 @@ bool DocumentObject::removeDynamicProperty(const char* name) return TransactionalObject::removeDynamicProperty(name); } +bool DocumentObject::renameDynamicProperty(Property* prop, const char* name) +{ + std::string oldName = prop->getName(); + bool renamed = TransactionalObject::renameDynamicProperty(prop, name); + if (renamed && _pDoc) { + _pDoc->renamePropertyOfObject(this, prop, oldName.c_str()); + } + return renamed; +} + App::Property* DocumentObject::addDynamicProperty(const char* type, const char* name, const char* group, diff --git a/src/App/DocumentObject.h b/src/App/DocumentObject.h index 6eefea054a..76e100591e 100644 --- a/src/App/DocumentObject.h +++ b/src/App/DocumentObject.h @@ -568,6 +568,8 @@ public: bool removeDynamicProperty(const char* prop) override; + bool renameDynamicProperty(Property *prop, const char *name) override; + App::Property* addDynamicProperty(const char* type, const char* name = nullptr, const char* group = nullptr, diff --git a/src/App/PropertyContainer.h b/src/App/PropertyContainer.h index c673c0b500..0404b886fa 100644 --- a/src/App/PropertyContainer.h +++ b/src/App/PropertyContainer.h @@ -548,7 +548,7 @@ public: * @return `true` if the property was renamed; `false` otherwise. * @throw Base::NameError If the new name is invalid or already exists. */ - bool renameDynamicProperty(Property *prop, const char *name) { + virtual bool renameDynamicProperty(Property *prop, const char *name) { return dynamicProps.renameDynamicProperty(prop,name); } diff --git a/src/App/Transactions.cpp b/src/App/Transactions.cpp index af83c79d9b..ba96d7c3ea 100644 --- a/src/App/Transactions.cpp +++ b/src/App/Transactions.cpp @@ -148,7 +148,8 @@ bool Transaction::hasObject(const TransactionalObject* Obj) const #endif } -void Transaction::addOrRemoveProperty(TransactionalObject* Obj, const Property* pcProp, bool add) +void Transaction::changeProperty(TransactionalObject* Obj, + std::function changeFunc) { auto& index = _Objects.get<1>(); auto pos = index.find(Obj); @@ -164,7 +165,21 @@ void Transaction::addOrRemoveProperty(TransactionalObject* Obj, const Property* index.emplace(Obj, To); } - To->addOrRemoveProperty(pcProp, add); + changeFunc(To); +} + +void Transaction::renameProperty(TransactionalObject* Obj, const Property* pcProp, const char* oldName) +{ + changeProperty(Obj, [pcProp, oldName](TransactionObject* to) { + to->renameProperty(pcProp, oldName); + }); +} + +void Transaction::addOrRemoveProperty(TransactionalObject* Obj, const Property* pcProp, bool add) +{ + changeProperty(Obj, [pcProp, add](TransactionObject* to) { + to->addOrRemoveProperty(pcProp, add); + }); } //************************************************************************** @@ -294,7 +309,13 @@ TransactionObject::TransactionObject() = default; TransactionObject::~TransactionObject() { for (auto& v : _PropChangeMap) { - delete v.second.property; + auto& data = v.second; + // If nameOrig is used, it means it is a transaction of a rename + // operation. This operation does not interact with v.second.property, + // so it should not be deleted in that case. + if (data.nameOrig.empty()) { + delete v.second.property; + } } } @@ -312,6 +333,15 @@ void TransactionObject::applyChn(Document& /*Doc*/, TransactionalObject* pcObj, auto& data = v.second; auto prop = const_cast(data.propertyOrig); + if (!data.nameOrig.empty()) { + // This means we are undoing/redoing a rename operation + Property* currentProp = pcObj->getDynamicPropertyByName(data.name.c_str()); + if (currentProp) { + pcObj->renameDynamicProperty(currentProp, data.nameOrig.c_str()); + } + continue; + } + if (!data.property) { // here means we are undoing/redoing and property add operation pcObj->removeDynamicProperty(v.second.name.c_str()); @@ -393,6 +423,21 @@ void TransactionObject::setProperty(const Property* pcProp) } } +void TransactionObject::renameProperty(const Property* pcProp, const char* oldName) +{ + if (!pcProp || !pcProp->getContainer()) { + return; + } + + auto& data = _PropChangeMap[pcProp->getID()]; + + if (data.name.empty()) { + static_cast(data) = + pcProp->getContainer()->getDynamicPropertyData(pcProp); + } + data.nameOrig = oldName; +} + void TransactionObject::addOrRemoveProperty(const Property* pcProp, bool add) { (void)add; diff --git a/src/App/Transactions.h b/src/App/Transactions.h index 494f59d162..eb57f2f888 100644 --- a/src/App/Transactions.h +++ b/src/App/Transactions.h @@ -81,12 +81,17 @@ public: bool isEmpty() const; /// check if this object is used in a transaction bool hasObject(const TransactionalObject* Obj) const; + void renameProperty(TransactionalObject* Obj, const Property* pcProp, const char* oldName); void addOrRemoveProperty(TransactionalObject* Obj, const Property* pcProp, bool add); void addObjectNew(TransactionalObject* Obj); void addObjectDel(const TransactionalObject* Obj); void addObjectChange(const TransactionalObject* Obj, const Property* Prop); +private: + void changeProperty(TransactionalObject* Obj, + std::function changeFunc); + private: int transID; using Info = std::pair; @@ -115,6 +120,7 @@ public: virtual void applyChn(Document& Doc, TransactionalObject* pcObj, bool Forward); void setProperty(const Property* pcProp); + void renameProperty(const Property* pcProp, const char* newName); void addOrRemoveProperty(const Property* pcProp, bool add); unsigned int getMemSize() const override; @@ -136,6 +142,8 @@ protected: { Base::Type propertyType; const Property* propertyOrig = nullptr; + // for property renaming + std::string nameOrig; }; std::unordered_map _PropChangeMap;