diff --git a/src/App/Application.cpp b/src/App/Application.cpp index 573d0331ae..05402889d2 100644 --- a/src/App/Application.cpp +++ b/src/App/Application.cpp @@ -38,6 +38,15 @@ # include # include # include +# include +# include +# include +# include +# include +# include +# include +# include +# include # include #endif @@ -80,6 +89,7 @@ #include #include #include +#include #include #include #include @@ -454,101 +464,88 @@ void Application::renameDocument(const char *OldName, const char *NewName) throw Base::RuntimeError("Renaming document internal name is no longer allowed!"); } -Document* Application::newDocument(const char * Name, const char * UserName, DocumentCreateFlags CreateFlags) +Document* Application::newDocument(const char * proposedName, const char * proposedLabel, DocumentCreateFlags CreateFlags) { - auto getNameAndLabel = [this](const char * Name, const char * UserName) -> std::tuple { - bool isDefaultName = Tools::isNullOrEmpty(Name); - - // get a valid name anyway! - if (isDefaultName) { - Name = "Unnamed"; - } - - std::string userName; - if (!Base::Tools::isNullOrEmpty(UserName)) { - userName = UserName; - } - else { - userName = isDefaultName ? QObject::tr("Unnamed").toStdString() : Name; - - std::vector names; - names.reserve(DocMap.size()); - for (const auto& pos : DocMap) { - names.emplace_back(pos.second->Label.getValue()); - } - - if (!names.empty()) { - userName = Base::Tools::getUniqueName(userName, names); - } - } - - return std::make_tuple(std::string(Name), userName); - }; - - auto tuple = getNameAndLabel(Name, UserName); - std::string name = std::get<0>(tuple); - std::string userName = std::get<1>(tuple); - name = getUniqueDocumentName(name.c_str(), CreateFlags.temporary); + bool isUsingDefaultName = Tools::isNullOrEmpty(proposedName); + // get a valid name anyway! + if (isUsingDefaultName) { + proposedName = "Unnamed"; + } + std::string name(getUniqueDocumentName(proposedName, CreateFlags.temporary)); // return the temporary document if it exists if (CreateFlags.temporary) { auto it = DocMap.find(name); - if (it != DocMap.end() && it->second->testStatus(Document::TempDoc)) + if (it != DocMap.end() && it->second->testStatus(Document::TempDoc)) { return it->second; + } } - // create the FreeCAD document - std::unique_ptr newDoc(new Document(name.c_str())); - newDoc->setStatus(Document::TempDoc, CreateFlags.temporary); + // Determine the document's Label + std::string label; + if (!Tools::isNullOrEmpty(proposedLabel)) { + // If a label is supplied it is used even if not unique + label = proposedLabel; + } + else { + label = isUsingDefaultName ? QObject::tr("Unnamed").toStdString() : proposedName; - auto oldActiveDoc = _pActiveDoc; - auto doc = newDoc.release(); // now owned by the Application + if (!DocMap.empty()) { + // The assumption here is that there are not many documents and + // documents are rarely created so the cost + // of building this manager each time is inconsequential + Base::UniqueNameManager names; + for (const auto& pos : DocMap) { + names.addExactName(pos.second->Label.getValue()); + } + + label = names.makeUniqueName(label); + } + } + // create the FreeCAD document + auto doc = new Document(name.c_str()); + doc->setStatus(Document::TempDoc, CreateFlags.temporary); // add the document to the internal list DocMap[name] = doc; - _pActiveDoc = doc; //NOLINTBEGIN // clang-format off // connect the signals to the application for the new document - _pActiveDoc->signalBeforeChange.connect(std::bind(&App::Application::slotBeforeChangeDocument, this, sp::_1, sp::_2)); - _pActiveDoc->signalChanged.connect(std::bind(&App::Application::slotChangedDocument, this, sp::_1, sp::_2)); - _pActiveDoc->signalNewObject.connect(std::bind(&App::Application::slotNewObject, this, sp::_1)); - _pActiveDoc->signalDeletedObject.connect(std::bind(&App::Application::slotDeletedObject, this, sp::_1)); - _pActiveDoc->signalBeforeChangeObject.connect(std::bind(&App::Application::slotBeforeChangeObject, this, sp::_1, sp::_2)); - _pActiveDoc->signalChangedObject.connect(std::bind(&App::Application::slotChangedObject, this, sp::_1, sp::_2)); - _pActiveDoc->signalRelabelObject.connect(std::bind(&App::Application::slotRelabelObject, this, sp::_1)); - _pActiveDoc->signalActivatedObject.connect(std::bind(&App::Application::slotActivatedObject, this, sp::_1)); - _pActiveDoc->signalUndo.connect(std::bind(&App::Application::slotUndoDocument, this, sp::_1)); - _pActiveDoc->signalRedo.connect(std::bind(&App::Application::slotRedoDocument, this, sp::_1)); - _pActiveDoc->signalRecomputedObject.connect(std::bind(&App::Application::slotRecomputedObject, this, sp::_1)); - _pActiveDoc->signalRecomputed.connect(std::bind(&App::Application::slotRecomputed, this, sp::_1)); - _pActiveDoc->signalBeforeRecompute.connect(std::bind(&App::Application::slotBeforeRecompute, this, sp::_1)); - _pActiveDoc->signalOpenTransaction.connect(std::bind(&App::Application::slotOpenTransaction, this, sp::_1, sp::_2)); - _pActiveDoc->signalCommitTransaction.connect(std::bind(&App::Application::slotCommitTransaction, this, sp::_1)); - _pActiveDoc->signalAbortTransaction.connect(std::bind(&App::Application::slotAbortTransaction, this, sp::_1)); - _pActiveDoc->signalStartSave.connect(std::bind(&App::Application::slotStartSaveDocument, this, sp::_1, sp::_2)); - _pActiveDoc->signalFinishSave.connect(std::bind(&App::Application::slotFinishSaveDocument, this, sp::_1, sp::_2)); - _pActiveDoc->signalChangePropertyEditor.connect(std::bind(&App::Application::slotChangePropertyEditor, this, sp::_1, sp::_2)); + doc->signalBeforeChange.connect(std::bind(&App::Application::slotBeforeChangeDocument, this, sp::_1, sp::_2)); + doc->signalChanged.connect(std::bind(&App::Application::slotChangedDocument, this, sp::_1, sp::_2)); + doc->signalNewObject.connect(std::bind(&App::Application::slotNewObject, this, sp::_1)); + doc->signalDeletedObject.connect(std::bind(&App::Application::slotDeletedObject, this, sp::_1)); + doc->signalBeforeChangeObject.connect(std::bind(&App::Application::slotBeforeChangeObject, this, sp::_1, sp::_2)); + doc->signalChangedObject.connect(std::bind(&App::Application::slotChangedObject, this, sp::_1, sp::_2)); + doc->signalRelabelObject.connect(std::bind(&App::Application::slotRelabelObject, this, sp::_1)); + doc->signalActivatedObject.connect(std::bind(&App::Application::slotActivatedObject, this, sp::_1)); + doc->signalUndo.connect(std::bind(&App::Application::slotUndoDocument, this, sp::_1)); + doc->signalRedo.connect(std::bind(&App::Application::slotRedoDocument, this, sp::_1)); + doc->signalRecomputedObject.connect(std::bind(&App::Application::slotRecomputedObject, this, sp::_1)); + doc->signalRecomputed.connect(std::bind(&App::Application::slotRecomputed, this, sp::_1)); + doc->signalBeforeRecompute.connect(std::bind(&App::Application::slotBeforeRecompute, this, sp::_1)); + doc->signalOpenTransaction.connect(std::bind(&App::Application::slotOpenTransaction, this, sp::_1, sp::_2)); + doc->signalCommitTransaction.connect(std::bind(&App::Application::slotCommitTransaction, this, sp::_1)); + doc->signalAbortTransaction.connect(std::bind(&App::Application::slotAbortTransaction, this, sp::_1)); + doc->signalStartSave.connect(std::bind(&App::Application::slotStartSaveDocument, this, sp::_1, sp::_2)); + doc->signalFinishSave.connect(std::bind(&App::Application::slotFinishSaveDocument, this, sp::_1, sp::_2)); + doc->signalChangePropertyEditor.connect(std::bind(&App::Application::slotChangePropertyEditor, this, sp::_1, sp::_2)); // clang-format on //NOLINTEND - // make sure that the active document is set in case no GUI is up - { - Base::PyGILStateLocker lock; - Py::Object active(_pActiveDoc->getPyObject(), true); - Py::Module("FreeCAD").setAttr(std::string("ActiveDocument"), active); - } + // (temporarily) make this the active document for the upcoming notifications. + // Signal NewDocument rather than ActiveDocument (which is what setActiveDocument would do) + auto oldActiveDoc = _pActiveDoc; + setActiveDocumentNoSignal(doc); + signalNewDocument(*doc, CreateFlags.createView); - signalNewDocument(*_pActiveDoc, CreateFlags.createView); - - // set the UserName after notifying all observers - _pActiveDoc->Label.setValue(userName); + doc->Label.setValue(label); // set the old document active again if the new is temporary - if (CreateFlags.temporary && oldActiveDoc) + if (CreateFlags.temporary && oldActiveDoc) { setActiveDocument(oldActiveDoc); - + } return doc; } @@ -618,29 +615,31 @@ std::vector Application::getDocuments() const return docs; } -std::string Application::getUniqueDocumentName(const char *Name, bool tempDoc) const +std::string Application::getUniqueDocumentName(const char* Name, bool tempDoc) const { - if (!Name || *Name == '\0') + if (!Name || *Name == '\0') { return {}; + } std::string CleanName = Base::Tools::getIdentifier(Name); // name in use? - std::map::const_iterator pos; - pos = DocMap.find(CleanName); + auto pos = DocMap.find(CleanName); if (pos == DocMap.end() || (tempDoc && pos->second->testStatus(Document::TempDoc))) { // if not, name is OK return CleanName; } - else { - std::vector names; - names.reserve(DocMap.size()); - for (pos = DocMap.begin(); pos != DocMap.end(); ++pos) { - if (!tempDoc || !pos->second->testStatus(Document::TempDoc)) - names.push_back(pos->first); + // The assumption here is that there are not many documents and + // documents are rarely created so the cost + // of building this manager each time is inconsequential + Base::UniqueNameManager names; + for (const auto& pos : DocMap) { + if (!tempDoc || !pos.second->testStatus(Document::TempDoc)) { + names.addExactName(pos.first); } - return Base::Tools::getUniqueName(CleanName, names); } + + return names.makeUniqueName(CleanName); } int Application::addPendingDocument(const char *FileName, const char *objName, bool allowPartial) @@ -1057,6 +1056,15 @@ Document* Application::getActiveDocument() const } void Application::setActiveDocument(Document* pDoc) +{ + setActiveDocumentNoSignal(pDoc); + + if (pDoc) { + signalActiveDocument(*pDoc); + } +} + +void Application::setActiveDocumentNoSignal(Document* pDoc) { _pActiveDoc = pDoc; @@ -1064,18 +1072,15 @@ void Application::setActiveDocument(Document* pDoc) if (pDoc) { Base::PyGILStateLocker lock; Py::Object active(pDoc->getPyObject(), true); - Py::Module("FreeCAD").setAttr(std::string("ActiveDocument"),active); + Py::Module("FreeCAD").setAttr(std::string("ActiveDocument"), active); } else { Base::PyGILStateLocker lock; - Py::Module("FreeCAD").setAttr(std::string("ActiveDocument"),Py::None()); + Py::Module("FreeCAD").setAttr(std::string("ActiveDocument"), Py::None()); } - - if (pDoc) - signalActiveDocument(*pDoc); } -void Application::setActiveDocument(const char *Name) +void Application::setActiveDocument(const char* Name) { // If no active document is set, resort to a default. if (*Name == '\0') { diff --git a/src/App/Application.h b/src/App/Application.h index 00c9a0c735..a91f2e105a 100644 --- a/src/App/Application.h +++ b/src/App/Application.h @@ -21,13 +21,17 @@ * * ***************************************************************************/ -#ifndef APP_APPLICATION_H -#define APP_APPLICATION_H +#ifndef SRC_APP_APPLICATION_H_ +#define SRC_APP_APPLICATION_H_ #include #include #include +#include +#include +#include +#include #include #include @@ -89,15 +93,15 @@ public: /** @name methods for document handling */ //@{ /** Creates a new document - * The first name is a the identifier and some kind of an internal (english) - * name. It has to be like an identifier in a programming language, with no - * spaces and not starting with a number. This name gets also forced to be unique - * in this Application. You can avoid the renaming by using getUniqueDocumentName() - * to get a unique name before calling newDoucument(). - * The second name is a UTF8 name of any kind. It's that name normally shown to - * the user and stored in the App::Document::Name property. + * @param proposedName: a prototype name used to create the permanent Name for the document. + * It is converted to be like an identifier in a programming language, + * with no spaces and not starting with a number. This name gets also forced to be unique + * in this Application. You can obtain the unique name using doc.getDocumentName + * on the returned document. + * @param proposedLabel: a UTF8 name of any kind. It's that name normally shown to + * the user and stored in the App::Document::Label property. */ - App::Document* newDocument(const char * Name=nullptr, const char * UserName=nullptr, + App::Document* newDocument(const char * proposedName=nullptr, const char * proposedLabel=nullptr, DocumentCreateFlags CreateFlags=DocumentCreateFlags()); /// Closes the document \a name and removes it from the application. bool closeDocument(const char* name); @@ -164,7 +168,7 @@ public: std::vector getDocuments() const; /// Set the active document void setActiveDocument(App::Document* pDoc); - void setActiveDocument(const char *Name); + void setActiveDocument(const char* Name); /// close all documents (without saving) void closeAllDocuments(); /// Add pending document to open together with the current opening document @@ -503,13 +507,16 @@ protected: }; private: - /// Constructor + /// Constructor. The passed configuration must last for the lifetime of the constructed Application + // NOLINTNEXTLINE(runtime/references) explicit Application(std::map &mConfig); /// Destructor virtual ~Application(); static void cleanupUnits(); + void setActiveDocumentNoSignal(App::Document* pDoc); + /** @name member for parameter */ //@{ static Base::Reference _pcSysParamMngr; @@ -654,4 +661,4 @@ inline App::Application &GetApplication(){ } // namespace App -#endif // APP_APPLICATION_H +#endif // SRC_APP_APPLICATION_H_ diff --git a/src/App/Document.cpp b/src/App/Document.cpp index 384e1b899a..f0c5d0c0c6 100644 --- a/src/App/Document.cpp +++ b/src/App/Document.cpp @@ -60,6 +60,17 @@ recompute path. Also, it enables more complicated dependencies beyond trees. #ifndef _PreComp_ #include #include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include #include #endif @@ -645,8 +656,10 @@ void Document::clearDocument() setStatus(Document::PartialDoc, false); d->clearRecomputeLog(); + d->objectLabelManager.clear(); d->objectArray.clear(); d->objectMap.clear(); + d->objectNameManager.clear(); d->objectIdMap.clear(); d->lastObjectId = 0; } @@ -2250,6 +2263,34 @@ bool Document::saveToFile(const char* filename) const return true; } +void Document::registerLabel(const std::string& newLabel) +{ + if (!newLabel.empty()) { + d->objectLabelManager.addExactName(newLabel); + } +} + +void Document::unregisterLabel(const std::string& oldLabel) +{ + if (!oldLabel.empty()) { + d->objectLabelManager.removeExactName(oldLabel); + } +} + +bool Document::containsLabel(const std::string& label) +{ + return d->objectLabelManager.containsName(label); +} + +std::string Document::makeUniqueLabel(const std::string& modelLabel) +{ + if (modelLabel.empty()) { + return {}; + } + + return d->objectLabelManager.makeUniqueName(modelLabel, 3); +} + bool Document::isAnyRestoring() { return globalIsRestoring; @@ -2276,7 +2317,9 @@ void Document::restore(const char* filename, setStatus(Document::PartialDoc, false); d->clearRecomputeLog(); + d->objectLabelManager.clear(); d->objectArray.clear(); + d->objectNameManager.clear(); d->objectMap.clear(); d->objectIdMap.clear(); d->lastObjectId = 0; @@ -3577,6 +3620,7 @@ DocumentObject* Document::addObject(const char* sType, // insert in the name map d->objectMap[ObjectName] = pcObject; + d->objectNameManager.addExactName(ObjectName); // generate object id and add to id map; pcObject->_Id = ++d->lastObjectId; d->objectIdMap[pcObject->_Id] = pcObject; @@ -3585,6 +3629,8 @@ DocumentObject* Document::addObject(const char* sType, pcObject->pcNameInDocument = &(d->objectMap.find(ObjectName)->first); // insert in the vector d->objectArray.push_back(pcObject); + // Register the current Label even though it is (probably) about to change + registerLabel(pcObject->Label.getStrValue()); // If we are restoring, don't set the Label object now; it will be restored later. This is to // avoid potential duplicate label conflicts later. @@ -3645,13 +3691,6 @@ Document::addObjects(const char* sType, const std::vector& objectNa return objects; } - // get all existing object names - std::vector reservedNames; - reservedNames.reserve(d->objectMap.size()); - for (const auto& pos : d->objectMap) { - reservedNames.push_back(pos.first); - } - for (auto it = objects.begin(); it != objects.end(); ++it) { auto index = std::distance(objects.begin(), it); App::DocumentObject* pcObject = *it; @@ -3666,29 +3705,19 @@ Document::addObjects(const char* sType, const std::vector& objectNa } } - // get unique name + // get unique name. We don't use getUniqueObjectName because it takes a char* not a std::string std::string ObjectName = objectNames[index]; if (ObjectName.empty()) { ObjectName = sType; } ObjectName = Base::Tools::getIdentifier(ObjectName); - if (d->objectMap.find(ObjectName) != d->objectMap.end()) { - // remove also trailing digits from clean name which is to avoid to create lengthy names - // like 'Box001001' - if (!testStatus(KeepTrailingDigits)) { - std::string::size_type index = ObjectName.find_last_not_of("0123456789"); - if (index + 1 < ObjectName.size()) { - ObjectName = ObjectName.substr(0, index + 1); - } - } - - ObjectName = Base::Tools::getUniqueName(ObjectName, reservedNames, 3); + if (d->objectNameManager.containsName(ObjectName)) { + ObjectName = d->objectNameManager.makeUniqueName(ObjectName, 3); } - reservedNames.push_back(ObjectName); - // insert in the name map d->objectMap[ObjectName] = pcObject; + d->objectNameManager.addExactName(ObjectName); // generate object id and add to id map; pcObject->_Id = ++d->lastObjectId; d->objectIdMap[pcObject->_Id] = pcObject; @@ -3697,6 +3726,8 @@ Document::addObjects(const char* sType, const std::vector& objectNa pcObject->pcNameInDocument = &(d->objectMap.find(ObjectName)->first); // insert in the vector d->objectArray.push_back(pcObject); + // Register the current Label even though it is about to change + registerLabel(pcObject->Label.getStrValue()); pcObject->Label.setValue(ObjectName); @@ -3757,6 +3788,7 @@ void Document::addObject(DocumentObject* pcObject, const char* pObjectName) // insert in the name map d->objectMap[ObjectName] = pcObject; + d->objectNameManager.addExactName(ObjectName); // generate object id and add to id map; if (!pcObject->_Id) { pcObject->_Id = ++d->lastObjectId; @@ -3767,6 +3799,8 @@ void Document::addObject(DocumentObject* pcObject, const char* pObjectName) pcObject->pcNameInDocument = &(d->objectMap.find(ObjectName)->first); // insert in the vector d->objectArray.push_back(pcObject); + // Register the current Label even though it is about to change + registerLabel(pcObject->Label.getStrValue()); pcObject->Label.setValue(ObjectName); @@ -3790,12 +3824,14 @@ void Document::_addObject(DocumentObject* pcObject, const char* pObjectName) { std::string ObjectName = getUniqueObjectName(pObjectName); d->objectMap[ObjectName] = pcObject; + d->objectNameManager.addExactName(ObjectName); // generate object id and add to id map; if (!pcObject->_Id) { pcObject->_Id = ++d->lastObjectId; } d->objectIdMap[pcObject->_Id] = pcObject; d->objectArray.push_back(pcObject); + registerLabel(pcObject->Label.getStrValue()); // cache the pointer to the name string in the Object (for performance of // DocumentObject::getNameInDocument()) pcObject->pcNameInDocument = &(d->objectMap.find(ObjectName)->first); @@ -3824,6 +3860,15 @@ void Document::_addObject(DocumentObject* pcObject, const char* pObjectName) signalActivatedObject(*pcObject); } +bool Document::containsObject(const DocumentObject* pcObject) const +{ + // We could look for the object in objectMap (keyed by object name), + // or search in objectArray (a O(n) vector search) but looking by Id + // in objectIdMap would be fastest. + auto found = d->objectIdMap.find(pcObject->getID()); + return found != d->objectIdMap.end() && found->second == pcObject; +} + /// Remove an object out of the document void Document::removeObject(const char* sName) { @@ -3911,6 +3956,7 @@ void Document::removeObject(const char* sName) } } + unregisterLabel(pos->second->Label.getStrValue()); for (std::vector::iterator obj = d->objectArray.begin(); obj != d->objectArray.end(); ++obj) { @@ -3924,6 +3970,7 @@ void Document::removeObject(const char* sName) if (tobedestroyed) { tobedestroyed->pcNameInDocument = nullptr; } + d->objectNameManager.removeExactName(pos->first); d->objectMap.erase(pos); } @@ -3993,6 +4040,8 @@ void Document::_removeObject(DocumentObject* pcObject) // remove from map pcObject->setStatus(ObjectStatus::Remove, false); // Unset the bit to be on the safe side d->objectIdMap.erase(pcObject->_Id); + d->objectNameManager.removeExactName(pos->first); + unregisterLabel(pos->second->Label.getStrValue()); d->objectMap.erase(pos); for (std::vector::iterator it = d->objectArray.begin(); @@ -4269,50 +4318,32 @@ const char* Document::getObjectName(DocumentObject* pFeat) const return nullptr; } -std::string Document::getUniqueObjectName(const char* Name) const +std::string Document::getUniqueObjectName(const char* proposedName) const { - if (!Name || *Name == '\0') { + if (!proposedName || *proposedName == '\0') { return {}; } - std::string CleanName = Base::Tools::getIdentifier(Name); + std::string cleanName = Base::Tools::getIdentifier(proposedName); - // name in use? - auto pos = d->objectMap.find(CleanName); - - if (pos == d->objectMap.end()) { - // if not, name is OK - return CleanName; - } - else { - // remove also trailing digits from clean name which is to avoid to create lengthy names - // like 'Box001001' - if (!testStatus(KeepTrailingDigits)) { - std::string::size_type index = CleanName.find_last_not_of("0123456789"); - if (index + 1 < CleanName.size()) { - CleanName = CleanName.substr(0, index + 1); - } - } - - std::vector names; - names.reserve(d->objectMap.size()); - for (pos = d->objectMap.begin(); pos != d->objectMap.end(); ++pos) { - names.push_back(pos->first); - } - return Base::Tools::getUniqueName(CleanName, names, 3); + if (!d->objectNameManager.containsName(cleanName)) { + // Not in use yet, name is OK + return cleanName; } + return d->objectNameManager.makeUniqueName(cleanName, 3); } -std::string Document::getStandardObjectName(const char* Name, int d) const + bool +Document::haveSameBaseName(const std::string& name, const std::string& label) { - std::vector mm = getObjects(); - std::vector labels; - labels.reserve(mm.size()); + // Both Labels and Names use the same decomposition rules for names, + // i.e. the default one supplied by UniqueNameManager, so we can use either + // of the name managers to do this test. + return d->objectNameManager.haveSameBaseName(name, label); +} - for (auto it : mm) { - std::string label = it->Label.getValue(); - labels.push_back(label); - } - return Base::Tools::getUniqueName(Name, labels, d); +std::string Document::getStandardObjectLabel(const char* modelName, int digitCount) const +{ + return d->objectLabelManager.makeUniqueName(modelName, digitCount); } std::vector Document::getDependingObjects() const diff --git a/src/App/Document.h b/src/App/Document.h index cdabf8efce..369e29377c 100644 --- a/src/App/Document.h +++ b/src/App/Document.h @@ -20,8 +20,8 @@ * * ***************************************************************************/ -#ifndef APP_DOCUMENT_H -#define APP_DOCUMENT_H +#ifndef SRC_APP_DOCUMENT_H_ +#define SRC_APP_DOCUMENT_H_ #include #include @@ -35,6 +35,9 @@ #include #include +#include +#include +#include #include namespace Base @@ -303,6 +306,10 @@ public: */ void addObject(DocumentObject*, const char* pObjectName = nullptr); + /// returns whether this is actually contains the DocumentObject. + /// Testing the DocumentObject's pDoc pointer is not sufficient because the object + /// removeObject and _removeObject leave _pDoc unchanged + bool containsObject(const DocumentObject*) const; /** Copy objects from another document to this document * @@ -334,11 +341,13 @@ public: /// Returns true if the DocumentObject is contained in this document bool isIn(const DocumentObject* pFeat) const; /// Returns a Name of an Object or 0 - const char* getObjectName(DocumentObject* pFeat) const; - /// Returns a Name of an Object or 0 - std::string getUniqueObjectName(const char* Name) const; - /// Returns a name of the form prefix_number. d specifies the number of digits. - std::string getStandardObjectName(const char* Name, int d) const; + const char *getObjectName(DocumentObject* pFeat) const; + /// Returns a Name for a new Object or empty if proposedName is null or empty. + std::string getUniqueObjectName(const char* proposedName) const; + /// Returns a name different from any of the Labels of any objects in this document, based on the given modelName. + std::string getStandardObjectLabel(const char* modelName, int d) const; + /// Determine if a given DocumentObject Name and a proposed Label are based on the same base name + bool haveSameBaseName(const std::string& name, const std::string& label); /// Returns a list of document's objects including the dependencies std::vector getDependingObjects() const; /// Returns a list of all Objects @@ -585,6 +594,11 @@ public: /// Indicate if there is any document restoring/importing static bool isAnyRestoring(); + void registerLabel(const std ::string& newLabel); + void unregisterLabel(const std::string& oldLabel); + bool containsLabel(const std::string& label); + std::string makeUniqueLabel(const std::string& modelLabel); + friend class Application; /// because of transaction handling friend class TransactionalObject; @@ -686,4 +700,4 @@ T* Document::addObject(const char* pObjectName, bool isNew, const char* viewType } // namespace App -#endif // APP_DOCUMENT_H +#endif // SRC_APP_DOCUMENT_H_ diff --git a/src/App/DocumentObject.cpp b/src/App/DocumentObject.cpp index 17f2b83ce8..ef4fdde3a9 100644 --- a/src/App/DocumentObject.cpp +++ b/src/App/DocumentObject.cpp @@ -25,6 +25,11 @@ #include "PreCompiled.h" #ifndef _PreComp_ #include +#include +#include +#include +#include +#include #endif #include @@ -814,6 +819,69 @@ void DocumentObject::onBeforeChange(const Property* prop) signalBeforeChange(*this, *prop); } +std::vector>> +DocumentObject::onProposedLabelChange(std::string& newLabel) +{ + // Note that this work can't be done in onBeforeChangeLabel because FeaturePython overrides this + // method and does not initially base-call it. + + // We re only called if the new label differs from the old one, and our code to check duplicates + // may not work if this is not the case. + std::string oldLabel = Label.getStrValue(); + assert(newLabel != oldLabel); + if (!isAttachedToDocument()) { + return {}; + } + App::Document* doc = getDocument(); + if (doc->isPerformingTransaction() + || (doc->testStatus(App::Document::Restoring) + && !doc->testStatus(App::Document::Importing))) { + return {}; + } + static ParameterGrp::handle _hPGrp; + if (!_hPGrp) { + _hPGrp = App::GetApplication().GetParameterGroupByPath("User parameter:BaseApp/Preferences/Document"); + } + if (doc && !newLabel.empty() && !_hPGrp->GetBool("DuplicateLabels") && !allowDuplicateLabel() + && doc->containsLabel(newLabel)) { + // We must ensure the Label is unique in the document (well, sort of...). + std::string objName = getNameInDocument(); + if (doc->haveSameBaseName(objName, newLabel)) { + // The base name of the proposed label equals the base name of the object Name, so we + // use the object Name, which could actually be identical to another object's Label, but + // probably isn't. + newLabel = objName; + } + else { + // Otherwise we generate a unique Label using newLabel as a prototype name. In doing so, + // we must also act as if the current value of the property is not an existing Label + // entry. + // We deregister the old label so it does not interfere with making the new label, + // and re-register it after. This is probably a bit less efficient that having a special + // make-unique-label-as-if-this-one-did-not-exist method, but such a method would be a real + // ugly wart. + doc->unregisterLabel(oldLabel); + newLabel = doc->makeUniqueLabel(newLabel); + doc->registerLabel(oldLabel); + } + } + + // Despite our efforts to make a unique label, onBeforeLabelChange can change it. + onBeforeChangeLabel(newLabel); + + if (oldLabel == newLabel || getDocument()->testStatus(App::Document::Restoring)) { + // Don't update label reference if we are restoring or if the label is unchanged. + // When importing (which also counts as restoring), it is possible the + // new object changes its label. However, we cannot update label + // references here, because object being restored is not based on + // dependency order. It can only be done in afterRestore(). + // + // See PropertyLinkBase::restoreLabelReference() for more details. + return {}; + } + return PropertyLinkBase::updateLabelReferences(this, newLabel.c_str()); +} + void DocumentObject::onEarlyChange(const Property* prop) { if (GetApplication().isClosingAll()) { @@ -836,6 +904,11 @@ void DocumentObject::onEarlyChange(const Property* prop) /// get called by the container when a Property was changed void DocumentObject::onChanged(const Property* prop) { + if (prop == &Label && _pDoc && _pDoc->containsObject(this) && oldLabel != Label.getStrValue()) { + _pDoc->unregisterLabel(oldLabel); + _pDoc->registerLabel(Label.getStrValue()); + } + if (isFreezed() && prop != &Visibility) { return; } diff --git a/src/App/DocumentObject.h b/src/App/DocumentObject.h index 72f78fe13d..c26a085731 100644 --- a/src/App/DocumentObject.h +++ b/src/App/DocumentObject.h @@ -22,8 +22,8 @@ ***************************************************************************/ -#ifndef APP_DOCUMENTOBJECT_H -#define APP_DOCUMENTOBJECT_H +#ifndef SRC_APP_DOCUMENTOBJECT_H_ +#define SRC_APP_DOCUMENTOBJECT_H_ #include #include @@ -33,6 +33,11 @@ #include #include +#include +#include +#include +#include +#include namespace Base { @@ -513,7 +518,14 @@ public: { return false; } - + /// Handle Label changes, including forcing unique label values, + /// signalling OnBeforeLabelChange, and arranging to update linked references, + /// on the assumption that after returning the label will indeed be changed to + /// the (altered) value of newLabel. + /// Returns a vector of referenging (linking) properties as produced by + /// PropertyLinkBase::updateLabelReferences which is needed for undo/redo purposes. + std::vector>> + onProposedLabelChange(std::string& newLabel); /*** Called to let object itself control relabeling * * @param newLabel: input as the new label, which can be modified by object itself @@ -765,10 +777,10 @@ protected: // attributes /// Old label; used for renaming expressions std::string oldLabel; +private: // pointer to the document name string (for performance) const std::string* pcNameInDocument {nullptr}; -private: // accessed by App::Document to record and restore the correct view provider type std::string _pcViewProviderName; @@ -787,4 +799,4 @@ private: } // namespace App -#endif // APP_DOCUMENTOBJECT_H +#endif // SRC_APP_DOCUMENTOBJECT_H_ diff --git a/src/App/DynamicProperty.cpp b/src/App/DynamicProperty.cpp index f79ecd5650..07921e87f4 100644 --- a/src/App/DynamicProperty.cpp +++ b/src/App/DynamicProperty.cpp @@ -22,9 +22,15 @@ #include "PreCompiled.h" +#ifndef _PreComp_ +#include +#include +#include +#endif #include #include +#include #include #include "DynamicProperty.h" @@ -70,7 +76,13 @@ void DynamicProperty::getPropertyNamedList( } } -void DynamicProperty::getPropertyMap(std::map& Map) const +void DynamicProperty::visitProperties(const std::function& visitor) const { + for (auto& v : props.get<0>()) { + visitor(v.property); + } +} + +void DynamicProperty::getPropertyMap(std::map& Map) const { for (auto& v : props.get<0>()) { Map[v.name] = v.property; @@ -296,27 +308,23 @@ bool DynamicProperty::removeDynamicProperty(const char* name) return false; } -std::string DynamicProperty::getUniquePropertyName(PropertyContainer& pc, const char* Name) const +std::string DynamicProperty::getUniquePropertyName(const PropertyContainer& pc, const char* Name) const { - std::string CleanName = Base::Tools::getIdentifier(Name); + std::string cleanName = Base::Tools::getIdentifier(Name); - // name in use? - std::map objectProps; - pc.getPropertyMap(objectProps); - auto pos = objectProps.find(CleanName); - - if (pos == objectProps.end()) { - // if not, name is OK - return CleanName; - } - else { - std::vector names; - names.reserve(objectProps.size()); - for (pos = objectProps.begin(); pos != objectProps.end(); ++pos) { - names.push_back(pos->first); - } - return Base::Tools::getUniqueName(CleanName, names); + // We test if the property already exists by finding it, which is not much more expensive than + // having a separate propertyExists(name) method. This avoids building the UniqueNameManager + // (which could also tell if the name exists) except in the relatively rare condition of + // the name already existing. + if (pc.getPropertyByName(cleanName.c_str()) == nullptr) { + return cleanName; } + Base::UniqueNameManager names; + // Build the index of existing names. + pc.visitProperties([&](Property* prop) { + names.addExactName(prop->getName()); + }); + return names.makeUniqueName(cleanName); } void DynamicProperty::save(const Property* prop, Base::Writer& writer) const @@ -335,7 +343,7 @@ void DynamicProperty::save(const Property* prop, Base::Writer& writer) const Property* DynamicProperty::restore(PropertyContainer& pc, const char* PropName, const char* TypeName, - Base::XMLReader& reader) + const Base::XMLReader& reader) { if (!reader.hasAttribute("group")) { return nullptr; diff --git a/src/App/DynamicProperty.h b/src/App/DynamicProperty.h index 29936fa665..1a0c0a42f9 100644 --- a/src/App/DynamicProperty.h +++ b/src/App/DynamicProperty.h @@ -21,12 +21,13 @@ ***************************************************************************/ -#ifndef APP_DYNAMICPROPERTY_H -#define APP_DYNAMICPROPERTY_H +#ifndef SRC_APP_DYNAMICPROPERTY_H_ +#define SRC_APP_DYNAMICPROPERTY_H_ #include #include #include +#include #include #include @@ -87,6 +88,8 @@ public: void getPropertyList(std::vector& List) const; /// get all properties with their names void getPropertyNamedList(std::vector>& List) const; + /// See PropertyContainer::visitProperties for semantics + void visitProperties(const std::function& visitor) const; /// Get all properties of the class (including parent) void getPropertyMap(std::map& Map) const; /// Find a dynamic property by its name @@ -166,7 +169,7 @@ public: Property* restore(PropertyContainer& pc, const char* PropName, const char* TypeName, - Base::XMLReader& reader); + const Base::XMLReader& reader); struct PropData { @@ -208,7 +211,7 @@ public: bool changeDynamicProperty(const Property* prop, const char* group, const char* doc); private: - std::string getUniquePropertyName(PropertyContainer& pc, const char* Name) const; + std::string getUniquePropertyName(const PropertyContainer& pc, const char* Name) const; private: bmi::multi_index_container< @@ -223,4 +226,4 @@ private: } // namespace App -#endif // APP_DYNAMICPROPERTY_H +#endif // SRC_APP_DYNAMICPROPERTY_H_ diff --git a/src/App/Extension.cpp b/src/App/Extension.cpp index c84e613de4..5ec6242da7 100644 --- a/src/App/Extension.cpp +++ b/src/App/Extension.cpp @@ -25,6 +25,9 @@ #ifndef _PreComp_ #include +#include +#include +#include #endif #include @@ -181,6 +184,10 @@ void Extension::extensionGetPropertyMap(std::map& Map) c extensionGetPropertyData().getPropertyMap(this, Map); } +void Extension::extensionVisitProperties(const std::function& visitor) const +{ + extensionGetPropertyData().visitProperties(this, visitor); +} void Extension::initExtensionSubclass(Base::Type& toInit, const char* ClassName, const char* ParentName, diff --git a/src/App/Extension.h b/src/App/Extension.h index 7c1545435e..8ee6b31098 100644 --- a/src/App/Extension.h +++ b/src/App/Extension.h @@ -21,8 +21,11 @@ ***************************************************************************/ -#ifndef APP_EXTENSION_H -#define APP_EXTENSION_H +#ifndef SRC_APP_EXTENSION_H_ +#define SRC_APP_EXTENSION_H_ + +#include +#include #include "PropertyContainer.h" #include @@ -46,7 +49,7 @@ private: \ #define EXTENSION_TYPESYSTEM_HEADER_WITH_OVERRIDE() \ public: \ static Base::Type getExtensionClassTypeId(void); \ - virtual Base::Type getExtensionTypeId(void) const override; \ + Base::Type getExtensionTypeId(void) const override; \ static void init(void);\ static void *create(void);\ private: \ @@ -97,7 +100,7 @@ private: \ EXTENSION_TYPESYSTEM_HEADER_WITH_OVERRIDE(); \ protected: \ static const App::PropertyData * extensionGetPropertyDataPtr(void); \ - virtual const App::PropertyData &extensionGetPropertyData(void) const override; \ + const App::PropertyData &extensionGetPropertyData(void) const override; \ private: \ static App::PropertyData propertyData @@ -254,6 +257,8 @@ public: virtual const char* extensionGetPropertyName(const Property* prop) const; /// get all properties of the class (including properties of the parent) virtual void extensionGetPropertyMap(std::map &Map) const; + /// See PropertyContainer::visitProperties for semantics + virtual void extensionVisitProperties(const std::function& visitor) const; /// get all properties of the class (including properties of the parent) virtual void extensionGetPropertyList(std::vector &List) const; @@ -327,6 +332,6 @@ private: // clang-format on -} //App +} // namespace App -#endif // APP_EXTENSION_H +#endif // SRC_APP_EXTENSION_H_ diff --git a/src/App/ExtensionContainer.cpp b/src/App/ExtensionContainer.cpp index 2fd642990d..9a002e576b 100644 --- a/src/App/ExtensionContainer.cpp +++ b/src/App/ExtensionContainer.cpp @@ -22,6 +22,11 @@ #include "PreCompiled.h" +#ifndef _PreComp_ +#include +#include +#include +#endif #include #include @@ -176,6 +181,13 @@ void ExtensionContainer::getPropertyMap(std::map& Map) c entry.second->extensionGetPropertyMap(Map); } } +void ExtensionContainer::visitProperties(const std::function& visitor) const +{ + App::PropertyContainer::visitProperties(visitor); + for (const auto &entry : _extensions) { + entry.second->extensionVisitProperties(visitor); + }; +} Property* ExtensionContainer::getPropertyByName(const char* name) const { diff --git a/src/App/ExtensionContainer.h b/src/App/ExtensionContainer.h index 7972ba4975..11b97c9054 100644 --- a/src/App/ExtensionContainer.h +++ b/src/App/ExtensionContainer.h @@ -21,8 +21,12 @@ ***************************************************************************/ -#ifndef APP_EXTENSIONCONTAINER_H -#define APP_EXTENSIONCONTAINER_H +#ifndef SRC_APP_EXTENSIONCONTAINER_H_ +#define SRC_APP_EXTENSIONCONTAINER_H_ + +#include +#include +#include #include "PropertyContainer.h" @@ -182,6 +186,8 @@ public: const char* getPropertyName(const Property* prop) const override; /// get all properties of the class (including properties of the parent) void getPropertyMap(std::map& Map) const override; + /// See PropertyContainer::visitProperties for semantics + void visitProperties(const std::function& visitor) const override; /// get all properties of the class (including properties of the parent) void getPropertyList(std::vector& List) const override; @@ -250,4 +256,4 @@ private: } // namespace App -#endif // APP_EXTENSIONCONTAINER_H +#endif // SRC_APP_EXTENSIONCONTAINER_H_ diff --git a/src/App/PropertyContainer.cpp b/src/App/PropertyContainer.cpp index 10100e65a3..9efdd7a0fd 100644 --- a/src/App/PropertyContainer.cpp +++ b/src/App/PropertyContainer.cpp @@ -22,6 +22,11 @@ #include "PreCompiled.h" +#ifndef _PreComp_ +#include +#include +#include +#endif #include #include @@ -92,6 +97,12 @@ void PropertyContainer::getPropertyList(std::vector &List) const getPropertyData().getPropertyList(this,List); } +void PropertyContainer::visitProperties(const std::function& visitor) const +{ + dynamicProps.visitProperties(visitor); + getPropertyData().visitProperties(this, visitor); +} + void PropertyContainer::getPropertyNamedList(std::vector > &List) const { dynamicProps.getPropertyNamedList(List); @@ -619,7 +630,15 @@ void PropertyData::getPropertyNamedList(OffsetBase offsetBase, } } - +void PropertyData::visitProperties(OffsetBase offsetBase, + const std::function& visitor) const +{ + merge(); + char* offset = offsetBase.getOffset(); + for (const auto& spec : propertyData.get<0>()) { + visitor(reinterpret_cast(spec.Offset + offset)); + }; +} /** \defgroup PropFrame Property framework \ingroup APP diff --git a/src/App/PropertyContainer.h b/src/App/PropertyContainer.h index 5a6b7647ae..c20cc2a9cf 100644 --- a/src/App/PropertyContainer.h +++ b/src/App/PropertyContainer.h @@ -21,11 +21,13 @@ ***************************************************************************/ -#ifndef APP_PROPERTYCONTAINER_H -#define APP_PROPERTYCONTAINER_H +#ifndef SRC_APP_PROPERTYCONTAINER_H_ +#define SRC_APP_PROPERTYCONTAINER_H_ #include #include +#include +#include #include #include "DynamicProperty.h" @@ -75,8 +77,12 @@ struct AppExport PropertyData //accepting void* struct OffsetBase { - OffsetBase(const App::PropertyContainer* container) : m_container(container) {}//explicit bombs - OffsetBase(const App::Extension* container) : m_container(container) {}//explicit bombs + // Lint wants these marked explicit, but they are currently used implicitly in enough + // places that I don't wnt to fix it. Instead we disable the Lint message. + // NOLINTNEXTLINE(runtime/explicit) + OffsetBase(const App::PropertyContainer* container) : m_container(container) {} + // NOLINTNEXTLINE(runtime/explicit) + OffsetBase(const App::Extension* container) : m_container(container) {} short int getOffsetTo(const App::Property* prop) const { auto *pt = (const char*)prop; @@ -134,6 +140,8 @@ struct AppExport PropertyData void getPropertyMap(OffsetBase offsetBase,std::map &Map) const; void getPropertyList(OffsetBase offsetBase,std::vector &List) const; void getPropertyNamedList(OffsetBase offsetBase, std::vector > &List) const; + /// See PropertyContainer::visitProperties for semantics + void visitProperties(OffsetBase offsetBase, const std::function& visitor) const; void merge(PropertyData *other=nullptr) const; void split(PropertyData *other); @@ -172,6 +180,11 @@ public: virtual void getPropertyMap(std::map &Map) const; /// get all properties of the class (including properties of the parent) virtual void getPropertyList(std::vector &List) const; + /// Call the given visitor for each property. The visiting order is undefined. + /// This method is necessary because PropertyContainer has no begin and end methods + /// and it is not practical to implement these. + /// What gets visited is undefined if the collection of Properties is changed during this call. + virtual void visitProperties(const std::function& visitor) const; /// get all properties with their names, may contain duplicates and aliases virtual void getPropertyNamedList(std::vector > &List) const; /// set the Status bit of all properties at once @@ -395,4 +408,4 @@ template<> void _class_::init(void){\ } // namespace App -#endif // APP_PROPERTYCONTAINER_H +#endif // SRC_APP_PROPERTYCONTAINER_H_ diff --git a/src/App/PropertyStandard.cpp b/src/App/PropertyStandard.cpp index 238df5d8b5..c89e4d1b6f 100644 --- a/src/App/PropertyStandard.cpp +++ b/src/App/PropertyStandard.cpp @@ -22,6 +22,16 @@ #include "PreCompiled.h" +#ifndef _PreComp_ +#include +#include +#include +#include +#include +#include +#include +#include +#endif #include #include @@ -1425,101 +1435,13 @@ void PropertyString::setValue(const char* newLabel) return; } - std::string _newLabel; - std::vector>> propChanges; - std::string label; + std::string label = newLabel; auto obj = dynamic_cast(getContainer()); bool commit = false; - if (obj && obj->isAttachedToDocument() && this == &obj->Label - && (!obj->getDocument()->testStatus(App::Document::Restoring) - || obj->getDocument()->testStatus(App::Document::Importing)) - && !obj->getDocument()->isPerformingTransaction()) { - // allow object to control label change - - static ParameterGrp::handle _hPGrp; - if (!_hPGrp) { - _hPGrp = GetApplication().GetUserParameter().GetGroup("BaseApp"); - _hPGrp = _hPGrp->GetGroup("Preferences")->GetGroup("Document"); - } - App::Document* doc = obj->getDocument(); - if (doc && !_hPGrp->GetBool("DuplicateLabels") && !obj->allowDuplicateLabel()) { - std::vector objectLabels; - std::vector::const_iterator it; - std::vector objs = doc->getObjects(); - bool match = false; - for (it = objs.begin(); it != objs.end(); ++it) { - if (*it == obj) { - continue; // don't compare object with itself - } - std::string objLabel = (*it)->Label.getValue(); - if (!match && objLabel == newLabel) { - match = true; - } - objectLabels.push_back(objLabel); - } - - // make sure that there is a name conflict otherwise we don't have to do anything - if (match && *newLabel) { - label = newLabel; - // remove number from end to avoid lengthy names - size_t lastpos = label.length() - 1; - while (label[lastpos] >= 48 && label[lastpos] <= 57) { - // if 'lastpos' becomes 0 then all characters are digits. In this case we use - // the complete label again - if (lastpos == 0) { - lastpos = label.length() - 1; - break; - } - lastpos--; - } - - bool changed = false; - label = label.substr(0, lastpos + 1); - if (label != obj->getNameInDocument() - && boost::starts_with(obj->getNameInDocument(), label)) { - // In case the label has the same base name as object's - // internal name, use it as the label instead. - const char* objName = obj->getNameInDocument(); - const char* c = &objName[lastpos + 1]; - for (; *c; ++c) { - if (*c < 48 || *c > 57) { - break; - } - } - if (*c == 0 - && std::find(objectLabels.begin(), - objectLabels.end(), - obj->getNameInDocument()) - == objectLabels.end()) { - label = obj->getNameInDocument(); - changed = true; - } - } - if (!changed) { - label = Base::Tools::getUniqueName(label, objectLabels, 3); - } - } - } - - if (label.empty()) { - label = newLabel; - } - obj->onBeforeChangeLabel(label); - newLabel = label.c_str(); - - if (!obj->getDocument()->testStatus(App::Document::Restoring)) { - // Only update label reference if we are not restoring. When - // importing (which also counts as restoring), it is possible the - // new object changes its label. However, we cannot update label - // references here, because object restoring is not based on - // dependency order. It can only be done in afterRestore(). - // - // See PropertyLinkBase::restoreLabelReference() for more details. - propChanges = PropertyLinkBase::updateLabelReferences(obj, newLabel); - } - + if (obj && this == &obj->Label) { + propChanges = obj->onProposedLabelChange(label); if (!propChanges.empty() && !GetApplication().getActiveTransaction()) { commit = true; std::ostringstream str; @@ -1529,11 +1451,11 @@ void PropertyString::setValue(const char* newLabel) } aboutToSetValue(); - _cValue = newLabel; + _cValue = label; hasSetValue(); for (auto& change : propChanges) { - change.first->Paste(*change.second.get()); + change.first->Paste(*change.second); } if (commit) { diff --git a/src/App/private/DocumentP.h b/src/App/private/DocumentP.h index 64d3b6697d..c7e73b6098 100644 --- a/src/App/private/DocumentP.h +++ b/src/App/private/DocumentP.h @@ -20,16 +20,22 @@ * * ***************************************************************************/ -#ifndef APP_DOCUMENTP_H -#define APP_DOCUMENTP_H +#ifndef SRC_APP_PRIVATE_DOCUMENTP_H_ +#define SRC_APP_PRIVATE_DOCUMENTP_H_ #ifdef _MSC_VER #pragma warning(disable : 4834) #endif +#include +#include +#include +#include + #include #include #include +#include #include #include #include @@ -65,6 +71,8 @@ struct DocumentP std::vector objectArray; std::unordered_set touchedObjs; std::unordered_map objectMap; + Base::UniqueNameManager objectNameManager; + Base::UniqueNameManager objectLabelManager; std::unordered_map objectIdMap; std::unordered_map partialLoadObjects; std::vector pendingRemove; @@ -129,6 +137,7 @@ struct DocumentP void clearDocument() { + objectLabelManager.clear(); objectArray.clear(); for (auto& v : objectMap) { v.second->setStatus(ObjectStatus::Destroy, true); @@ -136,6 +145,7 @@ struct DocumentP v.second = nullptr; } objectMap.clear(); + objectNameManager.clear(); objectIdMap.clear(); } @@ -161,4 +171,4 @@ struct DocumentP } // namespace App -#endif // APP_DOCUMENTP_H +#endif // SRC_APP_PRIVATE_DOCUMENTP_H_ diff --git a/src/Base/Reader.cpp b/src/Base/Reader.cpp index 69a8ecc23c..b63a294d23 100644 --- a/src/Base/Reader.cpp +++ b/src/Base/Reader.cpp @@ -24,7 +24,10 @@ #include "PreCompiled.h" #ifndef _PreComp_ -#include +#include +#include +#include +#include #include #include #endif @@ -465,14 +468,13 @@ const char* Base::XMLReader::addFile(const char* Name, Base::Persistence* Object temp.Object = Object; FileList.push_back(temp); - FileNames.push_back(temp.FileName); return Name; } -const std::vector& Base::XMLReader::getFilenames() const +bool Base::XMLReader::hasFilenames() const { - return FileNames; + return !FileList.empty(); } bool Base::XMLReader::hasReadFailed(const std::string& filename) const diff --git a/src/Base/Reader.h b/src/Base/Reader.h index f26c0617c8..ad4d5173ee 100644 --- a/src/Base/Reader.h +++ b/src/Base/Reader.h @@ -20,13 +20,14 @@ * * ***************************************************************************/ -#ifndef BASE_READER_H -#define BASE_READER_H +#ifndef SRC_BASE_READER_H_ +#define SRC_BASE_READER_H_ #include #include #include #include +#include #include #include @@ -250,8 +251,8 @@ public: const char* addFile(const char* Name, Base::Persistence* Object); /// process the requested file writes void readFiles(zipios::ZipInputStream& zipstream) const; - /// get all registered file names - const std::vector& getFilenames() const; + /// Returns whether reader has any registered filenames + bool hasFilenames() const; /// returns true if reading the file \a filename has failed bool hasReadFailed(const std::string& filename) const; bool isRegistered(Base::Persistence* Object) const; @@ -363,7 +364,6 @@ public: std::vector FileList; private: - std::vector FileNames; mutable std::vector FailedFiles; std::bitset<32> StatusBits; @@ -391,4 +391,4 @@ private: } // namespace Base -#endif +#endif // SRC_BASE_READER_H_ diff --git a/src/Base/Tools.cpp b/src/Base/Tools.cpp index 1c17850611..26a0867c8b 100644 --- a/src/Base/Tools.cpp +++ b/src/Base/Tools.cpp @@ -23,6 +23,8 @@ #include "PreCompiled.h" #ifndef _PreComp_ +#include +#include #include #include #endif @@ -31,130 +33,6 @@ #include "Interpreter.h" #include "Tools.h" -namespace Base -{ -struct string_comp -{ - // s1 and s2 must be numbers represented as string - bool operator()(const std::string& s1, const std::string& s2) - { - if (s1.size() < s2.size()) { - return true; - } - if (s1.size() > s2.size()) { - return false; - } - - return s1 < s2; - } - static std::string increment(const std::string& s) - { - std::string n = s; - int addcarry = 1; - for (std::string::reverse_iterator it = n.rbegin(); it != n.rend(); ++it) { - if (addcarry == 0) { - break; - } - int d = *it - 48; - d = d + addcarry; - *it = ((d % 10) + 48); - addcarry = d / 10; - } - if (addcarry > 0) { - std::string b; - b.resize(1); - b[0] = addcarry + 48; - n = b + n; - } - - return n; - } -}; - -class unique_name -{ -public: - unique_name(std::string name, const std::vector& names, int padding) - : base_name {std::move(name)} - , padding {padding} - { - removeDigitsFromEnd(); - findHighestSuffix(names); - } - - std::string get() const - { - return appendSuffix(); - } - -private: - void removeDigitsFromEnd() - { - std::string::size_type pos = base_name.find_last_not_of("0123456789"); - if (pos != std::string::npos && (pos + 1) < base_name.size()) { - num_suffix = base_name.substr(pos + 1); - base_name.erase(pos + 1); - } - } - - void findHighestSuffix(const std::vector& names) - { - for (const auto& name : names) { - if (name.substr(0, base_name.length()) == base_name) { // same prefix - std::string suffix(name.substr(base_name.length())); - if (!suffix.empty()) { - std::string::size_type pos = suffix.find_first_not_of("0123456789"); - if (pos == std::string::npos) { - num_suffix = std::max(num_suffix, suffix, Base::string_comp()); - } - } - } - } - } - - std::string appendSuffix() const - { - std::stringstream str; - str << base_name; - if (padding > 0) { - str.fill('0'); - str.width(padding); - } - str << Base::string_comp::increment(num_suffix); - return str.str(); - } - -private: - std::string num_suffix; - std::string base_name; - int padding; -}; - -} // namespace Base - -std::string -Base::Tools::getUniqueName(const std::string& name, const std::vector& names, int pad) -{ - if (names.empty()) { - return name; - } - - Base::unique_name unique(name, names, pad); - return unique.get(); -} - -std::string Base::Tools::addNumber(const std::string& name, unsigned int num, int d) -{ - std::stringstream str; - str << name; - if (d > 0) { - str.fill('0'); - str.width(d); - } - str << num; - return str.str(); -} - std::string Base::Tools::getIdentifier(const std::string& name) { if (name.empty()) { diff --git a/src/Base/Tools.h b/src/Base/Tools.h index 107be7617d..22a705708f 100644 --- a/src/Base/Tools.h +++ b/src/Base/Tools.h @@ -21,8 +21,8 @@ ***************************************************************************/ -#ifndef BASE_TOOLS_H -#define BASE_TOOLS_H +#ifndef SRC_BASE_TOOLS_H_ +#define SRC_BASE_TOOLS_H_ #ifndef FC_GLOBAL_H #include @@ -283,9 +283,6 @@ public: struct BaseExport Tools { - static std::string - getUniqueName(const std::string&, const std::vector&, int d = 0); - static std::string addNumber(const std::string&, unsigned int, int d = 0); static std::string getIdentifier(const std::string&); static std::wstring widen(const std::string& str); static std::string narrow(const std::wstring& str); @@ -341,4 +338,4 @@ struct BaseExport ZipTools } // namespace Base -#endif // BASE_TOOLS_H +#endif // SRC_BASE_TOOLS_H_ diff --git a/src/Base/UniqueNameManager.cpp b/src/Base/UniqueNameManager.cpp index e2941ffb45..34a050606a 100644 --- a/src/Base/UniqueNameManager.cpp +++ b/src/Base/UniqueNameManager.cpp @@ -136,8 +136,8 @@ Base::UniqueNameManager::decomposeName(const std::string& name) const digitCount, digitCount == 0 ? 0U : std::stoul(name.substr(name.crend() - digitsStart, digitCount))}; } -bool Base::UniqueNameManager::sameBaseName(const std::string& first, - const std::string& second) const +bool Base::UniqueNameManager::haveSameBaseName(const std::string& first, + const std::string& second) const { auto firstSuffixStart = getNameSuffixStartPosition(first); auto secondSuffixStart = getNameSuffixStartPosition(second); diff --git a/src/Base/UniqueNameManager.h b/src/Base/UniqueNameManager.h index ee17a39be8..c91c9772f1 100644 --- a/src/Base/UniqueNameManager.h +++ b/src/Base/UniqueNameManager.h @@ -124,7 +124,7 @@ public: virtual ~UniqueNameManager() = default; /// Check if two names are unique forms of the same base name - bool sameBaseName(const std::string& first, const std::string& second) const; + bool haveSameBaseName(const std::string& first, const std::string& second) const; /// Register a name in the collection. /// This collection acts as a multiset, so multiple registrations of the same diff --git a/src/Base/Writer.cpp b/src/Base/Writer.cpp index 91e67bbdb9..a068884923 100644 --- a/src/Base/Writer.cpp +++ b/src/Base/Writer.cpp @@ -22,6 +22,12 @@ #include "PreCompiled.h" +#ifndef _PreComp_ +#include +#include +#include +#include +#endif #include #include @@ -246,56 +252,19 @@ std::string Writer::addFile(const char* Name, const Base::Persistence* Object) assert(!isForceXML()); FileEntry temp; - temp.FileName = getUniqueFileName(Name); + temp.FileName = Name ? Name : ""; + if (FileNameManager.containsName(temp.FileName)) { + temp.FileName = FileNameManager.makeUniqueName(temp.FileName); + } temp.Object = Object; FileList.push_back(temp); - - FileNames.push_back(temp.FileName); + FileNameManager.addExactName(temp.FileName); // return the unique file name return temp.FileName; } -std::string Writer::getUniqueFileName(const char* Name) -{ - // name in use? - std::string CleanName = (Name ? Name : ""); - std::vector::const_iterator pos; - pos = find(FileNames.begin(), FileNames.end(), CleanName); - - if (pos == FileNames.end()) { - // if not, name is OK - return CleanName; - } - - std::vector names; - names.reserve(FileNames.size()); - FileInfo fi(CleanName); - CleanName = fi.fileNamePure(); - std::string ext = fi.extension(); - for (pos = FileNames.begin(); pos != FileNames.end(); ++pos) { - fi.setFile(*pos); - std::string FileName = fi.fileNamePure(); - if (fi.extension() == ext) { - names.push_back(FileName); - } - } - - std::stringstream str; - str << Base::Tools::getUniqueName(CleanName, names); - if (!ext.empty()) { - str << "." << ext; - } - - return str.str(); -} - -const std::vector& Writer::getFilenames() const -{ - return FileNames; -} - void Writer::incInd() { if (indent < 1020) { diff --git a/src/Base/Writer.h b/src/Base/Writer.h index 15d8368768..5211a0874b 100644 --- a/src/Base/Writer.h +++ b/src/Base/Writer.h @@ -20,8 +20,8 @@ * * ***************************************************************************/ -#ifndef BASE_WRITER_H -#define BASE_WRITER_H +#ifndef SRC_BASE_WRITER_H_ +#define SRC_BASE_WRITER_H_ #include @@ -32,6 +32,8 @@ #include +#include + #include "FileInfo.h" @@ -49,6 +51,24 @@ class Persistence; */ class BaseExport Writer { +private: + // This overrides UniqueNameManager's suffix-locating function so that the last '.' and + // everything after it is considered suffix. + class UniqueFileNameManager: public UniqueNameManager + { + protected: + std::string::const_reverse_iterator + getNameSuffixStartPosition(const std::string& name) const override + { + // This is an awkward way to do this, because the FileInfo class only yields pieces of + // the path, not delimiter positions. We can't just use fi.extension().size() because + // both "xyz" and "xyz." would yield three; we need the length of the extension + // *including its delimiter* so we use the length difference between the fileName and + // fileNamePure. + FileInfo fi(name); + return name.rbegin() + (fi.fileName().size() - fi.fileNamePure().size()); + } + }; public: Writer(); @@ -77,8 +97,6 @@ public: std::string addFile(const char* Name, const Base::Persistence* Object); /// process the requested file storing virtual void writeFiles() = 0; - /// get all registered file names - const std::vector& getFilenames() const; /// Set mode void setMode(const std::string& mode); /// Set modes @@ -144,14 +162,13 @@ public: std::string ObjectName; protected: - std::string getUniqueFileName(const char* Name); struct FileEntry { std::string FileName; const Base::Persistence* Object; }; std::vector FileList; - std::vector FileNames; + UniqueFileNameManager FileNameManager; std::vector Errors; std::set Modes; @@ -283,4 +300,4 @@ protected: } // namespace Base -#endif // BASE_WRITER_H +#endif // SRC_BASE_WRITER_H_ diff --git a/src/Gui/Document.cpp b/src/Gui/Document.cpp index 4a16841c4c..2edff11f6b 100644 --- a/src/Gui/Document.cpp +++ b/src/Gui/Document.cpp @@ -23,6 +23,14 @@ #include "PreCompiled.h" #ifndef _PreComp_ +# include +# include +# include +# include +# include +# include +# include +# include # include # include # include @@ -1938,8 +1946,9 @@ void Document::importObjects(const std::vector& obj, Base: localreader->readEndElement("Document"); // In the file GuiDocument.xml new data files might be added - if (!localreader->getFilenames().empty()) + if (localreader->hasFilenames()) { reader.initLocalReader(localreader); + } } void Document::slotFinishImportObjects(const std::vector &objs) { diff --git a/src/Gui/ViewProviderLink.cpp b/src/Gui/ViewProviderLink.cpp index 14b15f44ac..b4531a9fe5 100644 --- a/src/Gui/ViewProviderLink.cpp +++ b/src/Gui/ViewProviderLink.cpp @@ -25,6 +25,16 @@ #ifndef _PreComp_ # include # include +# include +# include +# include +# include +# include +# include +# include +# include +# include +# include # include # include # include @@ -41,7 +51,6 @@ # include # include # include -# include # include # include # include @@ -191,7 +200,7 @@ public: } } - LinkInfo(ViewProviderDocumentObject *vp) + explicit LinkInfo(ViewProviderDocumentObject *vp) :ref(0),pcLinked(vp) { FC_LOG("new link to " << pcLinked->getObject()->getFullName()); @@ -697,7 +706,7 @@ namespace Gui { px->release(); } -} +} // namespace Gui #endif //////////////////////////////////////////////////////////////////////////////////// @@ -3286,7 +3295,16 @@ void ViewProviderLink::getPropertyMap(std::map &Map) } } -void ViewProviderLink::getPropertyList(std::vector &List) const { +void ViewProviderLink::visitProperties(const std::function& visitor) const +{ + inherited::visitProperties(visitor); + if (childVp != nullptr) { + childVp->visitProperties(visitor); + } +} + +void ViewProviderLink::getPropertyList(std::vector& List) const +{ std::map Map; getPropertyMap(Map); List.reserve(List.size()+Map.size()); diff --git a/src/Gui/ViewProviderLink.h b/src/Gui/ViewProviderLink.h index c61fb19420..c84d31c504 100644 --- a/src/Gui/ViewProviderLink.h +++ b/src/Gui/ViewProviderLink.h @@ -20,10 +20,15 @@ * * ****************************************************************************/ -#ifndef GUI_VIEWPROVIDER_LINK_H -#define GUI_VIEWPROVIDER_LINK_H +#ifndef SRC_GUI_VIEWPROVIDER_LINK_H_ +#define SRC_GUI_VIEWPROVIDER_LINK_H_ #include +#include +#include +#include +#include +#include #include "Selection/SoFCUnifiedSelection.h" #include "ViewProviderDocumentObject.h" @@ -260,7 +265,9 @@ public: App::Property *getPropertyByName(const char* name) const override; void getPropertyMap(std::map &Map) const override; - void getPropertyList(std::vector &List) const override; + /// See PropertyContainer::visitProperties for semantics + void visitProperties(const std::function& visitor) const override; + void getPropertyList(std::vector& List) const override; ViewProviderDocumentObject *getLinkedViewProvider( std::string *subname=nullptr, bool recursive=false) const override; @@ -337,4 +344,4 @@ using ViewProviderLinkPython = ViewProviderFeaturePythonT; } //namespace Gui -#endif // GUI_VIEWPROVIDER_LINK_H +#endif // SRC_GUI_VIEWPROVIDER_LINK_H_ diff --git a/src/Mod/Spreadsheet/App/Sheet.cpp b/src/Mod/Spreadsheet/App/Sheet.cpp index f94a1d9642..1e309c1bff 100644 --- a/src/Mod/Spreadsheet/App/Sheet.cpp +++ b/src/Mod/Spreadsheet/App/Sheet.cpp @@ -27,6 +27,12 @@ #include #include #include +#include +#include +#include +#include +#include +#include #endif #include @@ -874,6 +880,17 @@ void Sheet::getPropertyNamedList(std::vector>& } } +void Sheet::visitProperties(const std::function& visitor) const +{ + DocumentObject::visitProperties(visitor); + for (const auto& v : cells.aliasProp) { + auto prop = getProperty(v.first); + if (prop != nullptr) { + visitor(prop); + } + }; +} + void Sheet::touchCells(Range range) { do { @@ -1136,7 +1153,7 @@ DocumentObjectExecReturn* Sheet::execute() catch (std::exception&) { // TODO: evaluate using a more specific exception (not_a_dag) // Cycle detected; flag all with errors Base::Console().Error("Cyclic dependency detected in spreadsheet : %s\n", - *pcNameInDocument); + getNameInDocument()); std::ostringstream ss; ss << "Cyclic dependency"; int count = 0; diff --git a/src/Mod/Spreadsheet/App/Sheet.h b/src/Mod/Spreadsheet/App/Sheet.h index 9b6ae0f6dc..9c5a259c1f 100644 --- a/src/Mod/Spreadsheet/App/Sheet.h +++ b/src/Mod/Spreadsheet/App/Sheet.h @@ -20,8 +20,8 @@ * * ***************************************************************************/ -#ifndef Spreadsheet_Spreadsheet_H -#define Spreadsheet_Spreadsheet_H +#ifndef SRC_MOD_SPREADSHEET_APP_SHEET_H_ +#define SRC_MOD_SPREADSHEET_APP_SHEET_H_ #ifdef signals #undef signals @@ -29,6 +29,10 @@ #endif #include +#include +#include +#include +#include #include #include @@ -189,6 +193,9 @@ public: void getPropertyNamedList(std::vector>& List) const override; + /// See PropertyContainer::visitProperties for semantics + void visitProperties(const std::function& visitor) const override; + short mustExecute() const override; App::DocumentObjectExecReturn* execute() override; @@ -307,4 +314,4 @@ using SheetPython = App::FeaturePythonT; } // namespace Spreadsheet -#endif // Spreadsheet_Spreadsheet_H +#endif // SRC_MOD_SPREADSHEET_APP_SHEET_H_ diff --git a/tests/src/Base/Tools.cpp b/tests/src/Base/Tools.cpp index f2385433d3..6bcb0b4c0b 100644 --- a/tests/src/Base/Tools.cpp +++ b/tests/src/Base/Tools.cpp @@ -1,47 +1,9 @@ #include #include #include +#include // NOLINTBEGIN(cppcoreguidelines-*,readability-*) -TEST(BaseToolsSuite, TestUniqueName1) -{ - EXPECT_EQ(Base::Tools::getUniqueName("Body", {}), "Body"); -} - -TEST(BaseToolsSuite, TestUniqueName2) -{ - EXPECT_EQ(Base::Tools::getUniqueName("Body", {"Body"}, 1), "Body1"); -} - -TEST(BaseToolsSuite, TestUniqueName3) -{ - EXPECT_EQ(Base::Tools::getUniqueName("Body", {"Body"}, 3), "Body001"); -} - -TEST(BaseToolsSuite, TestUniqueName4) -{ - EXPECT_EQ(Base::Tools::getUniqueName("Body", {"Body001"}, 3), "Body002"); -} - -TEST(BaseToolsSuite, TestUniqueName5) -{ - EXPECT_EQ(Base::Tools::getUniqueName("Body", {"Body", "Body001"}, 3), "Body002"); -} - -TEST(BaseToolsSuite, TestUniqueName6) -{ - EXPECT_EQ(Base::Tools::getUniqueName("Body001", {"Body", "Body001"}, 3), "Body002"); -} - -TEST(BaseToolsSuite, TestUniqueName7) -{ - EXPECT_EQ(Base::Tools::getUniqueName("Body001", {"Body"}, 3), "Body002"); -} - -TEST(BaseToolsSuite, TestUniqueName8) -{ - EXPECT_EQ(Base::Tools::getUniqueName("Body12345", {"Body"}, 3), "Body12346"); -} TEST(Tools, TestIota) {