diff --git a/src/App/Application.cpp b/src/App/Application.cpp index 9b9a0b7f56..56ef0a38b2 100644 --- a/src/App/Application.cpp +++ b/src/App/Application.cpp @@ -450,16 +450,41 @@ 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 * proposedName, const char * proposedLabel, bool createView, bool tempDoc) +Document* Application::newDocument(const char * Name, const char * UserName, bool createView, bool tempDoc) { - std::string name; - bool useDefaultName = (!proposedName || proposedName[0] == '\0'); - // get a valid name anyway! - if (useDefaultName) { - proposedName = "Unnamed"; - } + auto getNameAndLabel = [this](const char * Name, const char * UserName) -> std::tuple { + bool defaultName = (!Name || Name[0] == '\0'); - name = getUniqueDocumentName(proposedName, tempDoc); + // get a valid name anyway! + if (defaultName) { + Name = "Unnamed"; + } + + std::string userName; + if (UserName && UserName[0] != '\0') { + userName = UserName; + } + else { + userName = defaultName ? 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(), tempDoc); // return the temporary document if it exists if (tempDoc) { @@ -468,65 +493,53 @@ Document* Application::newDocument(const char * proposedName, const char * propo return it->second; } - // Determine the document's Label - std::string label; - if (proposedLabel && proposedLabel[0] != '\0') { - label = proposedLabel; - } - else { - label = useDefaultName ? QObject::tr("Unnamed").toStdString() : proposedName; - - 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 - Document* doc = new Document(name.c_str()); - doc->setStatus(Document::TempDoc, tempDoc); + std::unique_ptr newDoc(new Document(name.c_str())); + newDoc->setStatus(Document::TempDoc, tempDoc); + + auto oldActiveDoc = _pActiveDoc; + auto doc = newDoc.release(); // now owned by the Application // 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 - 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)); + _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)); // clang-format on //NOLINTEND - // (temporarily) make this the active document for the upcoming notifications. - // Signal NewDocument rather than ActiveDocument - auto oldActiveDoc = _pActiveDoc; - setActiveDocumentNoSignal(doc); - signalNewDocument(*doc, createView); + // 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); + } - doc->Label.setValue(label); + signalNewDocument(*_pActiveDoc, createView); + + // set the UserName after notifying all observers + _pActiveDoc->Label.setValue(userName); // set the old document active again if the new is temporary if (tempDoc && oldActiveDoc) @@ -616,17 +629,13 @@ std::string Application::getUniqueDocumentName(const char *Name, bool tempDoc) c return CleanName; } else { - // 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); - } + 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); } - - return names.makeUniqueName(CleanName); + return Base::Tools::getUniqueName(CleanName, names); } } @@ -1044,14 +1053,6 @@ Document* Application::getActiveDocument() const } void Application::setActiveDocument(Document* pDoc) -{ - setActiveDocumentNoSignal(pDoc); - - if (pDoc) - signalActiveDocument(*pDoc); -} - -void Application::setActiveDocumentNoSignal(Document* pDoc) { _pActiveDoc = pDoc; @@ -1059,15 +1060,18 @@ void Application::setActiveDocumentNoSignal(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 2a19bd80fd..e2a32adefe 100644 --- a/src/App/Application.h +++ b/src/App/Application.h @@ -160,7 +160,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 @@ -505,8 +505,6 @@ private: static void cleanupUnits(); - void setActiveDocumentNoSignal(App::Document* pDoc); - /** @name member for parameter */ //@{ static Base::Reference _pcSysParamMngr; diff --git a/src/App/Document.cpp b/src/App/Document.cpp index b834a4777b..131a18ba85 100644 --- a/src/App/Document.cpp +++ b/src/App/Document.cpp @@ -645,11 +645,8 @@ void Document::clearDocument() setStatus(Document::PartialDoc, false); d->clearRecomputeLog(); - d->objectLabelCounts.clear(); - d->objectLabelManager.clear(); d->objectArray.clear(); d->objectMap.clear(); - d->objectNameManager.clear(); d->objectIdMap.clear(); d->lastObjectId = 0; } @@ -2253,59 +2250,6 @@ bool Document::saveToFile(const char* filename) const return true; } -void Document::registerLabel(const std::string& newLabel) -{ - if (newLabel.empty()) { - return; - } - if (!d->objectLabelManager.containsName(newLabel)) { - // First occurrence of label. We make no entry in objectLabelCounts when the count is one. - d->objectLabelManager.addExactName(newLabel); - } - else { - auto it = d->objectLabelCounts.find(newLabel); - if (it != d->objectLabelCounts.end()) { - // There is a count already greater then one, so increment it - it->second++; - } - else { - // There is no count entry, which implies one, so register a count of two - d->objectLabelCounts.insert(std::pair(newLabel, 2)); - } - } -} - -void Document::unregisterLabel(const std::string& oldLabel) -{ - if (oldLabel.empty()) { - return; - } - auto it = d->objectLabelCounts.find(oldLabel); - if (it == d->objectLabelCounts.end()) { - // Missing count implies a count of one, or an unregistered name - d->objectLabelManager.removeExactName(oldLabel); - return; - } - if (--it->second == 1) { - // Decremented to one, remove the count entry - d->objectLabelCounts.erase(it); - } -} - -bool Document::containsLabel(const std::string& label) -{ - return d->objectLabelManager.containsName(label); -} - -std::string Document::makeUniqueLabel(const std::string& modelLabel) -{ - if (modelLabel.empty()) { - return std::string(); - } - - return d->objectLabelManager.makeUniqueName(modelLabel, 3); -} - bool Document::isAnyRestoring() { return globalIsRestoring; @@ -2332,10 +2276,7 @@ void Document::restore(const char* filename, setStatus(Document::PartialDoc, false); d->clearRecomputeLog(); - d->objectLabelCounts.clear(); - d->objectLabelManager.clear(); d->objectArray.clear(); - d->objectNameManager.clear(); d->objectMap.clear(); d->objectIdMap.clear(); d->lastObjectId = 0; @@ -3644,7 +3585,6 @@ 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; @@ -3653,8 +3593,6 @@ 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. @@ -3715,6 +3653,13 @@ 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; @@ -3729,19 +3674,29 @@ Document::addObjects(const char* sType, const std::vector& objectNa } } - // get unique name. We don't use getUniqueObjectName because it takes a char* not a std::string + // get unique name std::string ObjectName = objectNames[index]; if (ObjectName.empty()) { ObjectName = sType; } ObjectName = Base::Tools::getIdentifier(ObjectName); - if (d->objectNameManager.containsName(ObjectName)) { - ObjectName = d->objectNameManager.makeUniqueName(ObjectName, 3); + 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); } + 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; @@ -3750,8 +3705,6 @@ 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); @@ -3812,7 +3765,6 @@ 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; @@ -3823,8 +3775,6 @@ 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); @@ -3848,14 +3798,12 @@ 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); @@ -3884,15 +3832,6 @@ 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) { @@ -3980,7 +3919,6 @@ void Document::removeObject(const char* sName) } } - unregisterLabel(pos->second->Label.getStrValue()); for (std::vector::iterator obj = d->objectArray.begin(); obj != d->objectArray.end(); ++obj) { @@ -3994,7 +3932,6 @@ void Document::removeObject(const char* sName) if (tobedestroyed) { tobedestroyed->pcNameInDocument = nullptr; } - d->objectNameManager.removeExactName(pos->first); d->objectMap.erase(pos); } @@ -4064,8 +4001,6 @@ 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(); @@ -4342,29 +4277,50 @@ const char* Document::getObjectName(DocumentObject* pFeat) const return nullptr; } -std::string Document::getUniqueObjectName(const char* proposedName) const +std::string Document::getUniqueObjectName(const char* Name) const { - if (!proposedName || *proposedName == '\0') { + if (!Name || *Name == '\0') { return {}; } - std::string cleanName = Base::Tools::getIdentifier(proposedName); + std::string CleanName = Base::Tools::getIdentifier(Name); - if (!d->objectNameManager.containsName(cleanName)) { - // Not in use yet, name is OK - return cleanName; + // 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); } - return d->objectNameManager.makeUniqueName(cleanName, 3); } - std::tuple -Document::decomposeName(const std::string& name, std::string& baseName, std::string& nameExtension) +std::string Document::getStandardObjectName(const char* Name, int d) const { - return d->objectNameManager.decomposeName(name, baseName, nameExtension); -} + std::vector mm = getObjects(); + std::vector labels; + labels.reserve(mm.size()); -std::string Document::getStandardObjectLabel(const char* modelName, int digitCount) const -{ - return d->objectLabelManager.makeUniqueName(modelName, digitCount); + for (auto it : mm) { + std::string label = it->Label.getValue(); + labels.push_back(label); + } + return Base::Tools::getUniqueName(Name, labels, d); } std::vector Document::getDependingObjects() const diff --git a/src/App/Document.h b/src/App/Document.h index a740c2e530..36a878ee10 100644 --- a/src/App/Document.h +++ b/src/App/Document.h @@ -287,10 +287,6 @@ 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 * @@ -322,14 +318,11 @@ 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 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; - /// Break an object Name or Label into its base parts, returning tuple(digitCount, digitsValue) - std::tuple - decomposeName(const std::string& name, std::string& baseName, std::string& nameExtension); + 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; /// Returns a list of document's objects including the dependencies std::vector getDependingObjects() const; /// Returns a list of all Objects @@ -574,11 +567,6 @@ 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; diff --git a/src/App/DocumentObject.cpp b/src/App/DocumentObject.cpp index 8c2ad17b92..bd2b3b395e 100644 --- a/src/App/DocumentObject.cpp +++ b/src/App/DocumentObject.cpp @@ -814,74 +814,6 @@ 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); - std::string label; - if (!isAttachedToDocument() - || (getDocument()->testStatus(App::Document::Restoring) - && !getDocument()->testStatus(App::Document::Importing)) - || getDocument()->isPerformingTransaction()) { - return {}; - } - static ParameterGrp::handle _hPGrp; - if (!_hPGrp) { - _hPGrp = GetApplication().GetUserParameter().GetGroup("BaseApp"); - _hPGrp = _hPGrp->GetGroup("Preferences")->GetGroup("Document"); - } - App::Document* doc = getDocument(); - if (doc && newLabel.size() > 0 && !_hPGrp->GetBool("DuplicateLabels") && !allowDuplicateLabel() - && doc->containsLabel(newLabel)) { - // We must ensure the Label is unique in the document (well, sort of...). - // If the base name of the proposed label equals the base name of the object Name, we use - // the (uniquefied) object Name, which could actually be identical to another object's Label, - // but probably isn't. - // 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. - std::string objName = getNameInDocument(); - std::string objBaseName; - std::string objSuffix; - doc->decomposeName(objName, objBaseName, objSuffix); - std::string newBaseName; - std::string newSuffix; - doc->decomposeName(newLabel, newBaseName, newSuffix); - if (newBaseName == objBaseName && newSuffix == objSuffix) { - newLabel = objName; - } - else { - // 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()) { @@ -904,11 +836,6 @@ 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 666f066dfb..72f78fe13d 100644 --- a/src/App/DocumentObject.h +++ b/src/App/DocumentObject.h @@ -513,14 +513,7 @@ 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 @@ -772,10 +765,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; diff --git a/src/App/DynamicProperty.cpp b/src/App/DynamicProperty.cpp index c9bdb89981..f79ecd5650 100644 --- a/src/App/DynamicProperty.cpp +++ b/src/App/DynamicProperty.cpp @@ -70,13 +70,7 @@ void DynamicProperty::getPropertyNamedList( } } -void DynamicProperty::visitProperties(std::function visitor) const { - for (auto& v : props.get<0>()) { - visitor(v.property); - } -} - -void DynamicProperty::getPropertyMap(std::map& Map) const +void DynamicProperty::getPropertyMap(std::map& Map) const { for (auto& v : props.get<0>()) { Map[v.name] = v.property; @@ -304,21 +298,25 @@ bool DynamicProperty::removeDynamicProperty(const char* name) std::string DynamicProperty::getUniquePropertyName(PropertyContainer& pc, const char* Name) const { - std::string cleanName = Base::Tools::getIdentifier(Name); + std::string CleanName = Base::Tools::getIdentifier(Name); - // 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; + // 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); } - 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 diff --git a/src/App/DynamicProperty.h b/src/App/DynamicProperty.h index 0e24c63ca5..29936fa665 100644 --- a/src/App/DynamicProperty.h +++ b/src/App/DynamicProperty.h @@ -87,8 +87,6 @@ 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(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 diff --git a/src/App/Extension.cpp b/src/App/Extension.cpp index c942a0e5cb..c84e613de4 100644 --- a/src/App/Extension.cpp +++ b/src/App/Extension.cpp @@ -181,10 +181,6 @@ void Extension::extensionGetPropertyMap(std::map& Map) c extensionGetPropertyData().getPropertyMap(this, Map); } -void Extension::extensionVisitProperties(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 b4395d8704..7c1545435e 100644 --- a/src/App/Extension.h +++ b/src/App/Extension.h @@ -254,8 +254,6 @@ 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(std::function visitor) const; /// get all properties of the class (including properties of the parent) virtual void extensionGetPropertyList(std::vector &List) const; diff --git a/src/App/ExtensionContainer.cpp b/src/App/ExtensionContainer.cpp index e16f7bcac9..2fd642990d 100644 --- a/src/App/ExtensionContainer.cpp +++ b/src/App/ExtensionContainer.cpp @@ -176,13 +176,6 @@ void ExtensionContainer::getPropertyMap(std::map& Map) c entry.second->extensionGetPropertyMap(Map); } } -void ExtensionContainer::visitProperties(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 957c8a34d7..9fb5d6284f 100644 --- a/src/App/ExtensionContainer.h +++ b/src/App/ExtensionContainer.h @@ -182,8 +182,6 @@ 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(std::function visitor) const override; /// get all properties of the class (including properties of the parent) void getPropertyList(std::vector& List) const override; diff --git a/src/App/PropertyContainer.cpp b/src/App/PropertyContainer.cpp index 3b33044d76..10100e65a3 100644 --- a/src/App/PropertyContainer.cpp +++ b/src/App/PropertyContainer.cpp @@ -92,12 +92,6 @@ void PropertyContainer::getPropertyList(std::vector &List) const getPropertyData().getPropertyList(this,List); } -void PropertyContainer::visitProperties(std::function visitor) const -{ - dynamicProps.visitProperties(visitor); - getPropertyData().visitProperties(this, visitor); -} - void PropertyContainer::getPropertyNamedList(std::vector > &List) const { dynamicProps.getPropertyNamedList(List); @@ -625,15 +619,7 @@ void PropertyData::getPropertyNamedList(OffsetBase offsetBase, } } -void PropertyData::visitProperties(OffsetBase offsetBase, - 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 bbec1ab618..b8b5ac91ae 100644 --- a/src/App/PropertyContainer.h +++ b/src/App/PropertyContainer.h @@ -134,8 +134,6 @@ 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, std::function visitor) const; void merge(PropertyData *other=nullptr) const; void split(PropertyData *other); @@ -174,11 +172,6 @@ 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(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 diff --git a/src/App/PropertyStandard.cpp b/src/App/PropertyStandard.cpp index 1d91a97eda..696afa0ec4 100644 --- a/src/App/PropertyStandard.cpp +++ b/src/App/PropertyStandard.cpp @@ -1385,13 +1385,101 @@ void PropertyString::setValue(const char* newLabel) return; } + std::string _newLabel; + std::vector>> propChanges; - std::string label = newLabel; + std::string label; auto obj = dynamic_cast(getContainer()); bool commit = false; - if (obj && this == &obj->Label) { - propChanges = obj->onProposedLabelChange(label); + 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 (!propChanges.empty() && !GetApplication().getActiveTransaction()) { commit = true; std::ostringstream str; @@ -1401,7 +1489,7 @@ void PropertyString::setValue(const char* newLabel) } aboutToSetValue(); - _cValue = label; + _cValue = newLabel; hasSetValue(); for (auto& change : propChanges) { diff --git a/src/App/private/DocumentP.h b/src/App/private/DocumentP.h index a6006be3b8..64d3b6697d 100644 --- a/src/App/private/DocumentP.h +++ b/src/App/private/DocumentP.h @@ -30,7 +30,6 @@ #include #include #include -#include #include #include #include @@ -66,9 +65,6 @@ struct DocumentP std::vector objectArray; std::unordered_set touchedObjs; std::unordered_map objectMap; - Base::UniqueNameManager objectNameManager; - Base::UniqueNameManager objectLabelManager; - std::unordered_map objectLabelCounts; std::unordered_map objectIdMap; std::unordered_map partialLoadObjects; std::vector pendingRemove; @@ -133,8 +129,6 @@ struct DocumentP void clearDocument() { - objectLabelCounts.clear(); - objectLabelManager.clear(); objectArray.clear(); for (auto& v : objectMap) { v.second->setStatus(ObjectStatus::Destroy, true); @@ -142,7 +136,6 @@ struct DocumentP v.second = nullptr; } objectMap.clear(); - objectNameManager.clear(); objectIdMap.clear(); } diff --git a/src/Base/Reader.cpp b/src/Base/Reader.cpp index 2cbbf032d7..63b85bfa81 100644 --- a/src/Base/Reader.cpp +++ b/src/Base/Reader.cpp @@ -463,13 +463,14 @@ 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; } -bool Base::XMLReader::hasFilenames() const +const std::vector& Base::XMLReader::getFilenames() const { - return FileList.size() > 0; + return FileNames; } bool Base::XMLReader::hasReadFailed(const std::string& filename) const diff --git a/src/Base/Reader.h b/src/Base/Reader.h index 005114027d..a11b1966a2 100644 --- a/src/Base/Reader.h +++ b/src/Base/Reader.h @@ -251,8 +251,8 @@ public: const char* addFile(const char* Name, Base::Persistence* Object); /// process the requested file writes void readFiles(zipios::ZipInputStream& zipstream) const; - /// Returns whether reader has any registered filenames - bool hasFilenames() const; + /// get all registered file names + const std::vector& getFilenames() const; /// returns true if reading the file \a filename has failed bool hasReadFailed(const std::string& filename) const; bool isRegistered(Base::Persistence* Object) const; @@ -364,6 +364,7 @@ public: std::vector FileList; private: + std::vector FileNames; mutable std::vector FailedFiles; std::bitset<32> StatusBits; diff --git a/src/Base/Tools.cpp b/src/Base/Tools.cpp index 24c5856e77..71f6021bee 100644 --- a/src/Base/Tools.cpp +++ b/src/Base/Tools.cpp @@ -33,214 +33,130 @@ #include "Interpreter.h" #include "Tools.h" -void Base::UniqueNameManager::PiecewiseSparseIntegerSet::Add(uint value) +namespace Base { - etype newSpan(value, 1); - iterator above = Spans.lower_bound(newSpan); - if (above != Spans.end() && above->first <= value) { - // The found span includes value so there is nothing to do as it is already in the set. - return; +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); } - // Set below to the next span down, if any - iterator below; - if (above == Spans.begin()) { - below = Spans.end(); - } - else { - below = above; - --below; + std::string get() const + { + return appendSuffix(); } - if (above != Spans.end() && below != Spans.end() - && above->first - below->first + 1 == below->second) { - // below and above have a gap of exactly one between them, and this must be value - // so we coalesce the two spans (and the gap) into one. - newSpan = etype(below->first, below->second + above->second + 1); - Spans.erase(above); - above = Spans.erase(below); +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); + } } - if (below != Spans.end() && value - below->first == below->second) { - // value is adjacent to the end of below, so just expand below by one - newSpan = etype(below->first, below->second + 1); - above = Spans.erase(below); + + 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()); + } + } + } + } } - else if (above != Spans.end() && above->first - value == 1) { - // value is adjacent to the start of above, so juse expand above down by one - newSpan = etype(above->first - 1, above->second + 1); - above = Spans.erase(above); + + 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(); } - // else value is not adjacent to any existing span, so just make anew span for it - Spans.insert(above, newSpan); -} -void Base::UniqueNameManager::PiecewiseSparseIntegerSet::Remove(uint value) + +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) { - etype newSpan(value, 1); - iterator at = Spans.lower_bound(newSpan); - if (at == Spans.end() || at->first > value) { - // The found span does not include value so there is nothing to do, as it is already not in - // the set. - return; + if (names.empty()) { + return name; } - if (at->second == 1) { - // value is the only in this span, just remove the span - Spans.erase(at); - } - else if (at->first == value) { - // value is the first in this span, trim the lower end - etype replacement(at->first + 1, at->second - 1); - Spans.insert(Spans.erase(at), replacement); - } - else if (value - at->first == at->second - 1) { - // value is the last in this span, trim the upper end - etype replacement(at->first, at->second - 1); - Spans.insert(Spans.erase(at), replacement); - } - else { - // value is in the moddle of the span, so we must split it. - etype firstReplacement(at->first, value - at->first); - etype secondReplacement(value + 1, at->second - ((value + 1) - at->first)); - // Because erase returns the iterator after the erased element, and insert returns the - // iterator for the inserted item, we want to insert secondReplacement first. - Spans.insert(Spans.insert(Spans.erase(at), secondReplacement), firstReplacement); - } -} -bool Base::UniqueNameManager::PiecewiseSparseIntegerSet::Contains(uint value) const -{ - iterator at = Spans.lower_bound(etype(value, 1)); - return at != Spans.end() && at->first <= value; + + Base::unique_name unique(name, names, pad); + return unique.get(); } -std::tuple Base::UniqueNameManager::decomposeName(const std::string& name, - std::string& baseNameOut, - std::string& nameSuffixOut) const +std::string Base::Tools::addNumber(const std::string& name, unsigned int num, int d) { - auto suffixStart = std::make_reverse_iterator(GetNameSuffixStartPosition(name)); - nameSuffixOut = name.substr(name.crend() - suffixStart); - auto digitsStart = std::find_if_not(suffixStart, name.crend(), [](char c) { - return std::isdigit(c); - }); - baseNameOut = name.substr(0, name.crend() - digitsStart); - uint digitCount = digitsStart - suffixStart; - if (digitCount == 0) { - // No digits in name - return std::tuple {0, 0}; + std::stringstream str; + str << name; + if (d > 0) { + str.fill('0'); + str.width(d); } - else { - return std::tuple { - digitCount, - std::stoul(name.substr(name.crend() - digitsStart, digitCount))}; - } -} -void Base::UniqueNameManager::addExactName(const std::string& name) -{ - std::string baseName; - std::string nameSuffix; - uint digitCount; - uint digitsValue; - std::tie(digitCount, digitsValue) = decomposeName(name, baseName, nameSuffix); - baseName += nameSuffix; - auto baseNameEntry = UniqueSeeds.find(baseName); - if (baseNameEntry == UniqueSeeds.end()) { - // First use of baseName - baseNameEntry = - UniqueSeeds.emplace(baseName, std::vector()).first; - } - if (digitCount >= baseNameEntry->second.size()) { - // First use of this digitCount - baseNameEntry->second.resize(digitCount + 1); - } - PiecewiseSparseIntegerSet& baseNameAndDigitCountEntry = baseNameEntry->second[digitCount]; - // Name should not already be there - assert(!baseNameAndDigitCountEntry.Contains(digitsValue)); - baseNameAndDigitCountEntry.Add(digitsValue); -} -std::string Base::UniqueNameManager::makeUniqueName(const std::string& modelName, - std::size_t minDigits) const -{ - std::string namePrefix; - std::string nameSuffix; - decomposeName(modelName, namePrefix, nameSuffix); - std::string baseName = namePrefix + nameSuffix; - auto baseNameEntry = UniqueSeeds.find(baseName); - if (baseNameEntry == UniqueSeeds.end()) { - // First use of baseName, just return it with no unique digits - return baseName; - } - // We don't care about the digit count of the suggested name, we always use at least the most - // digits ever used before. - std::size_t digitCount = baseNameEntry->second.size() - 1; - uint digitsValue; - if (digitCount < minDigits) { - // Caller is asking for more digits than we have in any registered name. - // We start the longer digit string at 000...0001 even though we might have shorter strings - // with larger numeric values. - digitCount = minDigits; - digitsValue = 1; - } - else { - digitsValue = baseNameEntry->second[digitCount].Next(); - } - std::string digits = std::to_string(digitsValue); - if (digitCount > digits.size()) { - namePrefix += std::string(digitCount - digits.size(), '0'); - } - return namePrefix + digits + nameSuffix; + str << num; + return str.str(); } -void Base::UniqueNameManager::removeExactName(const std::string& name) -{ - std::string baseName; - std::string nameSuffix; - uint digitCount; - uint digitsValue; - std::tie(digitCount, digitsValue) = decomposeName(name, baseName, nameSuffix); - baseName += nameSuffix; - auto baseNameEntry = UniqueSeeds.find(baseName); - if (baseNameEntry == UniqueSeeds.end()) { - // name must not be registered, so nothing to do. - return; - } - auto& digitValueSets = baseNameEntry->second; - if (digitCount >= digitValueSets.size()) { - // First use of this digitCount, name must not be registered, so nothing to do. - return; - } - digitValueSets[digitCount].Remove(digitsValue); - // an element of digitValueSets may now be newly empty and so may other elements below it - // Prune off all such trailing empty entries. - auto lastNonemptyEntry = - std::find_if(digitValueSets.crbegin(), digitValueSets.crend(), [](auto& it) { - return it.Any(); - }); - if (lastNonemptyEntry == digitValueSets.crend()) { - // All entries are empty, so the entire baseName can be forgotten. - UniqueSeeds.erase(baseName); - } - else { - digitValueSets.resize(digitValueSets.crend() - lastNonemptyEntry); - } -} - -bool Base::UniqueNameManager::containsName(const std::string& name) const -{ - std::string baseName; - std::string nameSuffix; - uint digitCount; - uint digitsValue; - std::tie(digitCount, digitsValue) = decomposeName(name, baseName, nameSuffix); - baseName += nameSuffix; - auto baseNameEntry = UniqueSeeds.find(baseName); - if (baseNameEntry == UniqueSeeds.end()) { - // base name is not registered - return false; - } - if (digitCount >= baseNameEntry->second.size()) { - // First use of this digitCount, name must not be registered, so not in collection - return false; - } - return baseNameEntry->second[digitCount].Contains(digitsValue); -} 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 1295475ba1..baf61e872e 100644 --- a/src/Base/Tools.h +++ b/src/Base/Tools.h @@ -33,7 +33,6 @@ #include #include #include -#include #include #include @@ -265,100 +264,11 @@ public: // ---------------------------------------------------------------------------- - -class BaseExport UniqueNameManager -{ -protected: - // This method returns the position of the start of the suffix (or name.cend() if no - // suffix). It must return the same suffix lentgh (name.size() - returnValue) for both - // unique names (one containing digits) and the corresponding base name (with no digits). - virtual std::string::const_iterator GetNameSuffixStartPosition(const std::string& name) const - { - return name.cend(); - } - -private: - class PiecewiseSparseIntegerSet - { - public: - PiecewiseSparseIntegerSet() - {} - - private: - // Each pair being represents the span of integers from lowest to - // (lowest+count-1) inclusive - using etype = std::pair; - // This span comparer class is analogous to std::less and treats overlapping spans as being - // neither greater nor less than each other - class comparer - { - public: - bool operator()(const etype& lhs, const etype& rhs) const - { - // The equality case here is when lhs is below and directly adjacent to rhs. - return rhs.first - lhs.first >= lhs.second; - } - }; - // Spans is the set of spans. Adjacent spans are coalesced so there are always gaps between - // the entries. - std::set Spans; - using iterator = typename std::set::iterator; - using const_iterator = typename std::set::const_iterator; - - public: - void Add(uint value); - void Remove(uint value); - bool Contains(uint value) const; - bool Any() const - { - return Spans.size() != 0; - } - void Clear() - { - Spans.clear(); - } - uint Next() const - { - if (Spans.size() == 0) { - return 0; - } - iterator last = Spans.end(); - --last; - return last->first + last->second; - } - }; - // Keyed as UniqueSeeds[baseName][digitCount][digitValue] iff that seed is taken. - // We need the double-indexing so that Name01 and Name001 can both be indexed, although we only - // ever allocate off the longest for each name i.e. UniqueSeeds[baseName].size()-1 digits. - std::map> UniqueSeeds; - -public: - std::tuple decomposeName(const std::string& name, - std::string& baseNameOut, - std::string& nameSuffixOut) const; - - UniqueNameManager() - {} - - // Register a name in the collection. It is an error (detected only by assertions) to register a - // name more than once. The effect if undetected is that the second registration will have no - // effect - void addExactName(const std::string& name); - std::string makeUniqueName(const std::string& modelName, std::size_t minDigits = 0) const; - - // Remove a registered name so it can be generated again. - // Nothing happens if you try to remove a non-registered name. - void removeExactName(const std::string& name); - - bool containsName(const std::string& name) const; - - void clear() - { - UniqueSeeds.clear(); - } -}; 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); diff --git a/src/Base/Writer.cpp b/src/Base/Writer.cpp index 17aa16d507..d80af5a743 100644 --- a/src/Base/Writer.cpp +++ b/src/Base/Writer.cpp @@ -247,19 +247,56 @@ std::string Writer::addFile(const char* Name, const Base::Persistence* Object) assert(!isForceXML()); FileEntry temp; - temp.FileName = Name ? Name : ""; - if (FileNameManager.containsName(temp.FileName)) { - temp.FileName = FileNameManager.makeUniqueName(temp.FileName); - } + temp.FileName = getUniqueFileName(Name); temp.Object = Object; FileList.push_back(temp); - FileNameManager.addExactName(temp.FileName); + + FileNames.push_back(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 0b59815814..5ca61be9df 100644 --- a/src/Base/Writer.h +++ b/src/Base/Writer.h @@ -39,8 +39,6 @@ #include #include -#include - #include "FileInfo.h" @@ -58,24 +56,6 @@ class Persistence; */ class BaseExport Writer { -private: - // This overrides UniqueNameManager's suffix-locating function so thet the last '.' and - // everything after it is considered suffix. - class UniqueFileNameManager: public UniqueNameManager - { - protected: - virtual std::string::const_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.end() - (fi.fileName().size() - fi.fileNamePure().size()); - } - }; public: Writer(); @@ -104,6 +84,8 @@ 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 @@ -169,13 +151,14 @@ public: std::string ObjectName; protected: + std::string getUniqueFileName(const char* Name); struct FileEntry { std::string FileName; const Base::Persistence* Object; }; std::vector FileList; - UniqueFileNameManager FileNameManager; + std::vector FileNames; std::vector Errors; std::set Modes; diff --git a/src/Gui/Document.cpp b/src/Gui/Document.cpp index e7df35552d..d75604d5b0 100644 --- a/src/Gui/Document.cpp +++ b/src/Gui/Document.cpp @@ -1938,7 +1938,7 @@ void Document::importObjects(const std::vector& obj, Base: localreader->readEndElement("Document"); // In the file GuiDocument.xml new data files might be added - if (localreader->hasFilenames()) + if (!localreader->getFilenames().empty()) reader.initLocalReader(localreader); } diff --git a/src/Gui/ViewProviderLink.cpp b/src/Gui/ViewProviderLink.cpp index 39447e29be..a2cad7a67f 100644 --- a/src/Gui/ViewProviderLink.cpp +++ b/src/Gui/ViewProviderLink.cpp @@ -3408,16 +3408,7 @@ void ViewProviderLink::getPropertyMap(std::map &Map) } } -void ViewProviderLink::visitProperties(std::function visitor) const -{ - inherited::visitProperties(visitor); - if (childVp != nullptr) { - childVp->visitProperties(visitor); - } -} - -void ViewProviderLink::getPropertyList(std::vector& List) const -{ +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 67c355991c..fc296f2561 100644 --- a/src/Gui/ViewProviderLink.h +++ b/src/Gui/ViewProviderLink.h @@ -265,9 +265,7 @@ public: App::Property *getPropertyByName(const char* name) const override; void getPropertyMap(std::map &Map) const override; - /// See PropertyContainer::visitProperties for semantics - void visitProperties(std::function visitor) const override; - void getPropertyList(std::vector& List) const override; + void getPropertyList(std::vector &List) const override; ViewProviderDocumentObject *getLinkedViewProvider( std::string *subname=nullptr, bool recursive=false) const override; diff --git a/src/Mod/Spreadsheet/App/Sheet.cpp b/src/Mod/Spreadsheet/App/Sheet.cpp index 71d32b42e5..0171fd1eee 100644 --- a/src/Mod/Spreadsheet/App/Sheet.cpp +++ b/src/Mod/Spreadsheet/App/Sheet.cpp @@ -872,17 +872,6 @@ void Sheet::getPropertyNamedList(std::vector>& } } -void Sheet::visitProperties(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 { @@ -1145,7 +1134,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", - getNameInDocument()); + *pcNameInDocument); 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 217f4f7efe..9b6ae0f6dc 100644 --- a/src/Mod/Spreadsheet/App/Sheet.h +++ b/src/Mod/Spreadsheet/App/Sheet.h @@ -189,9 +189,6 @@ public: void getPropertyNamedList(std::vector>& List) const override; - /// See PropertyContainer::visitProperties for semantics - void visitProperties(std::function visitor) const override; - short mustExecute() const override; App::DocumentObjectExecReturn* execute() override; diff --git a/tests/src/Base/Tools.cpp b/tests/src/Base/Tools.cpp index fa219b31ee..f2385433d3 100644 --- a/tests/src/Base/Tools.cpp +++ b/tests/src/Base/Tools.cpp @@ -5,58 +5,42 @@ // NOLINTBEGIN(cppcoreguidelines-*,readability-*) TEST(BaseToolsSuite, TestUniqueName1) { - EXPECT_EQ(Base::UniqueNameManager().makeUniqueName("Body"), "Body"); + EXPECT_EQ(Base::Tools::getUniqueName("Body", {}), "Body"); } TEST(BaseToolsSuite, TestUniqueName2) { - Base::UniqueNameManager manager; - manager.addExactName("Body"); - EXPECT_EQ(manager.makeUniqueName("Body", 1), "Body1"); + EXPECT_EQ(Base::Tools::getUniqueName("Body", {"Body"}, 1), "Body1"); } TEST(BaseToolsSuite, TestUniqueName3) { - Base::UniqueNameManager manager; - manager.addExactName("Body"); - EXPECT_EQ(manager.makeUniqueName("Body", 3), "Body001"); + EXPECT_EQ(Base::Tools::getUniqueName("Body", {"Body"}, 3), "Body001"); } TEST(BaseToolsSuite, TestUniqueName4) { - Base::UniqueNameManager manager; - manager.addExactName("Body001"); - EXPECT_EQ(manager.makeUniqueName("Body", 3), "Body002"); + EXPECT_EQ(Base::Tools::getUniqueName("Body", {"Body001"}, 3), "Body002"); } TEST(BaseToolsSuite, TestUniqueName5) { - Base::UniqueNameManager manager; - manager.addExactName("Body"); - manager.addExactName("Body001"); - EXPECT_EQ(manager.makeUniqueName("Body", 3), "Body002"); + EXPECT_EQ(Base::Tools::getUniqueName("Body", {"Body", "Body001"}, 3), "Body002"); } TEST(BaseToolsSuite, TestUniqueName6) { - Base::UniqueNameManager manager; - manager.addExactName("Body"); - manager.addExactName("Body001"); - EXPECT_EQ(manager.makeUniqueName("Body001", 3), "Body002"); + EXPECT_EQ(Base::Tools::getUniqueName("Body001", {"Body", "Body001"}, 3), "Body002"); } TEST(BaseToolsSuite, TestUniqueName7) { - Base::UniqueNameManager manager; - manager.addExactName("Body"); - EXPECT_EQ(manager.makeUniqueName("Body001", 3), "Body001"); + EXPECT_EQ(Base::Tools::getUniqueName("Body001", {"Body"}, 3), "Body002"); } TEST(BaseToolsSuite, TestUniqueName8) { - Base::UniqueNameManager manager; - manager.addExactName("Body"); - EXPECT_EQ(manager.makeUniqueName("Body12345", 3), "Body001"); + EXPECT_EQ(Base::Tools::getUniqueName("Body12345", {"Body"}, 3), "Body12346"); } TEST(Tools, TestIota)