From fc79bad535e4e8cc8d6c979d8ebccb6c545700e0 Mon Sep 17 00:00:00 2001 From: Mahmoud Badri Date: Wed, 15 May 2024 19:38:37 +0300 Subject: [PATCH] QmlDesigner: Remove Model::generateIdFromName() - Added UniqueName::getId() - Cleaned up generateNewId() and made it use UniqueName::getId() - Moved UniqueName to designercore Also reversed how the predicate of UniqueName::get() works. Change-Id: I89c50f7d80610243f56be165b1495ef428da457c Reviewed-by: Reviewed-by: Qt CI Patch Build Bot Reviewed-by: Miikka Heikkinen --- src/plugins/qmldesigner/CMakeLists.txt | 8 +- .../assetslibrary/assetslibrarymodel.cpp | 2 +- .../assetslibrary/assetslibrarywidget.cpp | 2 +- .../collectioneditor/collectionview.cpp | 2 +- .../contentlibraryusermodel.cpp | 6 +- .../contentlibrary/contentlibraryview.cpp | 4 +- .../materialeditor/materialeditorview.cpp | 4 +- .../qmldesigner/designercore/include/model.h | 4 +- .../qmldesigner/designercore/model/model.cpp | 93 ++------------ .../qmldesigner/designercore/uniquename.cpp | 121 ++++++++++++++++++ .../qmldesigner/designercore/uniquename.h | 32 +++++ src/plugins/qmldesigner/utils/uniquename.cpp | 93 -------------- src/plugins/qmldesigner/utils/uniquename.h | 20 --- .../tests/testdesignercore/CMakeLists.txt | 2 + 14 files changed, 181 insertions(+), 212 deletions(-) create mode 100644 src/plugins/qmldesigner/designercore/uniquename.cpp create mode 100644 src/plugins/qmldesigner/designercore/uniquename.h delete mode 100644 src/plugins/qmldesigner/utils/uniquename.cpp delete mode 100644 src/plugins/qmldesigner/utils/uniquename.h diff --git a/src/plugins/qmldesigner/CMakeLists.txt b/src/plugins/qmldesigner/CMakeLists.txt index 87f6e9c66d1..2d67fa360f7 100644 --- a/src/plugins/qmldesigner/CMakeLists.txt +++ b/src/plugins/qmldesigner/CMakeLists.txt @@ -46,7 +46,6 @@ add_qtc_library(QmlDesignerUtils STATIC hdrimage.cpp hdrimage.h ktximage.cpp ktximage.h imageutils.cpp imageutils.h - uniquename.cpp uniquename.h qmldesignerutils_global.h ) @@ -96,10 +95,9 @@ add_qtc_library(QmlDesignerCore STATIC ${CMAKE_CURRENT_LIST_DIR}/designercore/include SOURCES_PREFIX ${CMAKE_CURRENT_LIST_DIR}/designercore SOURCES - rewritertransaction.cpp - rewritertransaction.h - generatedcomponentutils.cpp - generatedcomponentutils.h + rewritertransaction.cpp rewritertransaction.h + generatedcomponentutils.cpp generatedcomponentutils.h + uniquename.cpp uniquename.h ) extend_qtc_library(QmlDesignerCore diff --git a/src/plugins/qmldesigner/components/assetslibrary/assetslibrarymodel.cpp b/src/plugins/qmldesigner/components/assetslibrary/assetslibrarymodel.cpp index d3a701aa5f2..36299e586bf 100644 --- a/src/plugins/qmldesigner/components/assetslibrary/assetslibrarymodel.cpp +++ b/src/plugins/qmldesigner/components/assetslibrary/assetslibrarymodel.cpp @@ -5,6 +5,7 @@ #include #include +#include #include @@ -12,7 +13,6 @@ #include #include #include -#include #include #include diff --git a/src/plugins/qmldesigner/components/assetslibrary/assetslibrarywidget.cpp b/src/plugins/qmldesigner/components/assetslibrary/assetslibrarywidget.cpp index e63ac9b0d5a..11439409a71 100644 --- a/src/plugins/qmldesigner/components/assetslibrary/assetslibrarywidget.cpp +++ b/src/plugins/qmldesigner/components/assetslibrary/assetslibrarywidget.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -28,7 +29,6 @@ #include #include #include -#include #include #include diff --git a/src/plugins/qmldesigner/components/collectioneditor/collectionview.cpp b/src/plugins/qmldesigner/components/collectioneditor/collectionview.cpp index f8382b19897..8a9e6a8276b 100644 --- a/src/plugins/qmldesigner/components/collectioneditor/collectionview.cpp +++ b/src/plugins/qmldesigner/components/collectioneditor/collectionview.cpp @@ -242,7 +242,7 @@ void CollectionView::addResource(const QUrl &url, const QString &name) VariantProperty nameProperty = resourceNode.variantProperty("objectName"); sourceProperty.setValue(sourceAddress); nameProperty.setValue(name); - resourceNode.setIdWithoutRefactoring(model()->generateIdFromName(name, "model")); + resourceNode.setIdWithoutRefactoring(model()->generateNewId(name, "model")); rootModelNode().defaultNodeAbstractProperty().reparentHere(resourceNode); }); } diff --git a/src/plugins/qmldesigner/components/contentlibrary/contentlibraryusermodel.cpp b/src/plugins/qmldesigner/components/contentlibrary/contentlibraryusermodel.cpp index ca90c9c4517..7f664f0388d 100644 --- a/src/plugins/qmldesigner/components/contentlibrary/contentlibraryusermodel.cpp +++ b/src/plugins/qmldesigner/components/contentlibrary/contentlibraryusermodel.cpp @@ -13,10 +13,10 @@ #include #include #include +#include #include #include -#include #include #include @@ -318,11 +318,11 @@ QPair ContentLibraryUserModel::getUniqueLibItemNames(const QSt baseQml.prepend("My"); QString uniqueQml = UniqueName::get(baseQml, [&] (const QString &name) { - return !itemQmls.contains(name); + return itemQmls.contains(name); }); QString uniqueIcon = UniqueName::get(defaultName, [&] (const QString &name) { - return !itemIcons.contains(name); + return itemIcons.contains(name); }); return {uniqueQml + ".qml", uniqueIcon + ".png"}; diff --git a/src/plugins/qmldesigner/components/contentlibrary/contentlibraryview.cpp b/src/plugins/qmldesigner/components/contentlibrary/contentlibraryview.cpp index 8e5b5c4e3b3..0dbc3a4da71 100644 --- a/src/plugins/qmldesigner/components/contentlibrary/contentlibraryview.cpp +++ b/src/plugins/qmldesigner/components/contentlibrary/contentlibraryview.cpp @@ -779,7 +779,7 @@ ModelNode ContentLibraryView::createMaterial(const TypeName &typeName) QString newName = QString::fromUtf8(typeName).replace(rgx, " \\1\\2").trimmed(); if (newName.endsWith(" Material")) newName.chop(9); // remove trailing " Material" - QString newId = model()->generateIdFromName(newName, "material"); + QString newId = model()->generateNewId(newName, "material"); newMatNode.setIdWithRefactoring(newId); VariantProperty objNameProp = newMatNode.variantProperty("objectName"); @@ -805,7 +805,7 @@ ModelNode ContentLibraryView::createMaterial(const NodeMetaInfo &metaInfo) QString newName = QString::fromLatin1(metaInfo.simplifiedTypeName()).replace(rgx, " \\1\\2").trimmed(); if (newName.endsWith(" Material")) newName.chop(9); // remove trailing " Material" - QString newId = model()->generateIdFromName(newName, "material"); + QString newId = model()->generateNewId(newName, "material"); newMatNode.setIdWithRefactoring(newId); VariantProperty objNameProp = newMatNode.variantProperty("objectName"); diff --git a/src/plugins/qmldesigner/components/materialeditor/materialeditorview.cpp b/src/plugins/qmldesigner/components/materialeditor/materialeditorview.cpp index 02d9333e09c..21114267cbb 100644 --- a/src/plugins/qmldesigner/components/materialeditor/materialeditorview.cpp +++ b/src/plugins/qmldesigner/components/materialeditor/materialeditorview.cpp @@ -961,7 +961,7 @@ void MaterialEditorView::renameMaterial(ModelNode &material, const QString &newN return; executeInTransaction(__FUNCTION__, [&] { - material.setIdWithRefactoring(model()->generateIdFromName(newName, "material")); + material.setIdWithRefactoring(model()->generateNewId(newName, "material")); VariantProperty objNameProp = material.variantProperty("objectName"); objNameProp.setValue(newName); @@ -998,7 +998,7 @@ void MaterialEditorView::duplicateMaterial(const ModelNode &material) QString newName = sourceMat.modelNode().variantProperty("objectName").value().toString() + " copy"; VariantProperty objNameProp = duplicateMatNode.variantProperty("objectName"); objNameProp.setValue(newName); - duplicateMatNode.setIdWithoutRefactoring(model()->generateIdFromName(newName, "material")); + duplicateMatNode.setIdWithoutRefactoring(model()->generateNewId(newName, "material")); // sync properties. Only the base state is duplicated. const QList props = material.properties(); diff --git a/src/plugins/qmldesigner/designercore/include/model.h b/src/plugins/qmldesigner/designercore/include/model.h index 74c5697990c..5529064284e 100644 --- a/src/plugins/qmldesigner/designercore/include/model.h +++ b/src/plugins/qmldesigner/designercore/include/model.h @@ -255,10 +255,8 @@ public: bool hasId(const QString &id) const; bool hasImport(const QString &importUrl) const; - QString generateNewId(const QString &prefixName, - const QString &fallbackPrefix = "element", + QString generateNewId(const QString &prefixName, const QString &fallbackPrefix = "element", std::optional> isDuplicate = {}) const; - QString generateIdFromName(const QString &name, const QString &fallbackId = "element") const; void startDrag(QMimeData *mimeData, const QPixmap &icon); void endDrag(); diff --git a/src/plugins/qmldesigner/designercore/model/model.cpp b/src/plugins/qmldesigner/designercore/model/model.cpp index 23871b66699..d95d3e294bb 100644 --- a/src/plugins/qmldesigner/designercore/model/model.cpp +++ b/src/plugins/qmldesigner/designercore/model/model.cpp @@ -9,7 +9,6 @@ #include "../projectstorage/sourcepath.h" #include "../projectstorage/sourcepathcache.h" #include "abstractview.h" -#include "auxiliarydataproperties.h" #include "internalbindingproperty.h" #include "internalnodeabstractproperty.h" #include "internalnodelistproperty.h" @@ -33,6 +32,8 @@ #include "signalhandlerproperty.h" #include "variantproperty.h" +#include + #include #include @@ -46,8 +47,6 @@ #include #include -#include - /*! \defgroup CoreModel */ @@ -1864,90 +1863,22 @@ bool Model::hasImport(const QString &importUrl) const }); } -static QString firstCharToLower(const QString &string) -{ - QString resultString = string; - - if (!resultString.isEmpty()) - resultString[0] = resultString.at(0).toLower(); - - return resultString; -} - -QString Model::generateNewId(const QString &prefixName, - const QString &fallbackPrefix, +QString Model::generateNewId(const QString &prefixName, const QString &fallbackPrefix, std::optional> isDuplicate) const { - // First try just the prefixName without number as postfix, then continue with 2 and further - // as postfix until id does not already exist. - // Properties of the root node are not allowed for ids, because they are available in the - // complete context without qualification. + QString newId = prefixName; - int counter = 0; + if (newId.isEmpty()) + newId = fallbackPrefix; - static const QRegularExpression nonWordCharsRegex("\\W"); - QString newBaseId = firstCharToLower(prefixName); - newBaseId.remove(nonWordCharsRegex); - - if (!newBaseId.isEmpty()) { - QChar firstChar = newBaseId.at(0); - if (firstChar.isDigit()) - newBaseId.prepend('_'); - } else { - newBaseId = fallbackPrefix; - } - - QString newId = newBaseId; - - if (!isDuplicate.has_value()) + if (!isDuplicate.has_value()) // TODO: to be removed separately to not break build isDuplicate = std::bind(&Model::hasId, this, std::placeholders::_1); - while (!ModelNode::isValidId(newId) || isDuplicate.value()(newId) - || d->rootNode()->property(newId.toUtf8())) { - ++counter; - newId = QStringView(u"%1%2").arg(firstCharToLower(newBaseId)).arg(counter); - } - - return newId; -} - -// Generate a unique camelCase id from a name -// note: this methods does the same as generateNewId(). The 2 methods should be merged into one -QString Model::generateIdFromName(const QString &name, const QString &fallbackId) const -{ - QString newId; - if (name.isEmpty()) { - newId = fallbackId; - } else { - // convert to camel case - QStringList nameWords = name.split(" "); - nameWords[0] = nameWords[0].at(0).toLower() + nameWords[0].mid(1); - for (int i = 1; i < nameWords.size(); ++i) - nameWords[i] = nameWords[i].at(0).toUpper() + nameWords[i].mid(1); - newId = nameWords.join(""); - - // if id starts with a number prepend an underscore - if (newId.at(0).isDigit()) - newId.prepend('_'); - } - - // If the new id is not valid (e.g. qml keyword match), try fixing it by prepending underscore - if (!ModelNode::isValidId(newId)) - newId.prepend("_"); - - QRegularExpression rgx("\\d+$"); // matches a number at the end of a string - while (hasId(newId)) { // id exists - QRegularExpressionMatch match = rgx.match(newId); - if (match.hasMatch()) { // ends with a number, increment it - QString numStr = match.captured(); - int num = numStr.toInt() + 1; - newId = newId.mid(0, match.capturedStart()) + QString::number(num); - } else { - newId.append('1'); - } - } - - return newId; + return UniqueName::getId(prefixName, [&] (const QString &id) { + // Properties of the root node are not allowed for ids, because they are available in the + // complete context without qualification. + return isDuplicate.value()(id) || d->rootNode()->property(id.toUtf8()); + }); } void Model::startDrag(QMimeData *mimeData, const QPixmap &icon) diff --git a/src/plugins/qmldesigner/designercore/uniquename.cpp b/src/plugins/qmldesigner/designercore/uniquename.cpp new file mode 100644 index 00000000000..7bfa2c74303 --- /dev/null +++ b/src/plugins/qmldesigner/designercore/uniquename.cpp @@ -0,0 +1,121 @@ +// Copyright (C) 2024 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 + +#include "uniquename.h" + +#include +#include + +namespace QmlDesigner { + +/** + * @brief Generates a unique name based on the provided name. + * + * This method iteratively generates a name by appending suffixes until a unique name is found. + * The uniqueness of the generated name is determined by the provided predicate function. + * + * @param name The original name to be made unique. + * @param predicate A function that checks if a name exists. Returns true if the name exists, + * false if name is unique. + * @return A unique name derived from the provided name. + */ +QString UniqueName::get(const QString &name, std::function predicate) +{ + if (!predicate(name)) + return name; + + // match prefix and number (including zero padding) parts + static QRegularExpression rgx("(\\D*?)(\\d+)$"); + QRegularExpressionMatch match = rgx.match(name); + + QString prefix; + int number = 0; + int padding = 0; + + if (match.hasMatch()) { + // Split the name into prefix and number + prefix = match.captured(1); + QString numberStr = match.captured(2); + number = numberStr.toInt(); + padding = numberStr.size(); + } else { + prefix = name; + } + + QString nameTemplate = "%1%2"; + QString newName; + do { + newName = nameTemplate.arg(prefix).arg(++number, padding, 10, QChar('0')); + } while (predicate(newName)); + + return newName; +} + +/** + * @brief Generates a unique path based on the provided path. If the path belongs to a file, the + * filename or if it's a directory, the directory name will be adjusted to ensure uniqueness. + * + * This method appends a numerical suffix (or increment it if it exists) to the filename or + * directory name if necessary to make it unique. + * + * @param path The original path to be made unique. + * @return A unique path derived from the provided path. + */ +QString UniqueName::getPath(const QString &path) +{ + // Remove the trailing slash if it exists (otherwise QFileInfo::path() returns empty) + QString adjustedPath = path; + if (adjustedPath.endsWith('/')) + adjustedPath.chop(1); + + QFileInfo fileInfo = QFileInfo(adjustedPath); + QString baseName = fileInfo.baseName(); + QString suffix = fileInfo.completeSuffix(); + if (!suffix.isEmpty()) + suffix.prepend('.'); + + QString parentDir = fileInfo.path(); + QString pathTemplate = parentDir + "/%1" + suffix; + + QString uniqueBaseName = UniqueName::get(baseName, [&] (const QString &currName) { + return !QFileInfo::exists(pathTemplate.arg(currName)); + }); + + return pathTemplate.arg(uniqueBaseName); +} + +/** + * @brief Generates a unique ID based on the provided id + * + * This works similar to get() with additional restrictions: + * - Removes non-Latin1 characters + * - Removes spaces + * - Ensures the first letter is lowercase + * - Converts spaces to camel case + * - Prepends an underscore if id starts with a number or is a reserved word + * + * @param id The original id to be made unique. + * @return A unique Id (when predicate() returns false) + */ +QString UniqueName::getId(const QString &id, std::function predicate) +{ + // remove non word (non A-Z, a-z, 0-9) characters + static const QRegularExpression nonWordCharsRgx("\\W"); + QString newId = id.simplified(); + newId.remove(nonWordCharsRgx); + + // convert to camel case + QStringList idParts = newId.split(" "); + idParts[0] = idParts[0].at(0).toLower() + idParts[0].mid(1); + for (int i = 1; i < idParts.size(); ++i) + idParts[i] = idParts[i].at(0).toUpper() + idParts[i].mid(1); + newId = idParts.join(""); + + // prepend _ if starts with a digit or invalid id (such as reserved words) + if (newId.at(0).isDigit() || std::binary_search(keywords.begin(), keywords.end(), newId)) + newId.prepend('_'); + + return UniqueName::get(newId, predicate); +} + +} // namespace QmlDesigner diff --git a/src/plugins/qmldesigner/designercore/uniquename.h b/src/plugins/qmldesigner/designercore/uniquename.h new file mode 100644 index 00000000000..3d6bc6c0e36 --- /dev/null +++ b/src/plugins/qmldesigner/designercore/uniquename.h @@ -0,0 +1,32 @@ +// Copyright (C) 2024 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 + +#pragma once + +#include + +#include +#include + +namespace QmlDesigner { + +class QMLDESIGNERCORE_EXPORT UniqueName +{ +public: + static QString get(const QString &name, std::function predicate); + static QString getPath(const QString &path); + static QString getId(const QString &id, std::function predicate); + +private: + static inline const QStringList keywords { + "anchors", "as", "baseState", "border", "bottom", "break", "case", "catch", "clip", "color", + "continue", "data", "debugger", "default", "delete", "do", "else", "enabled", "finally", + "flow", "focus", "font", "for", "function", "height", "if", "import", "in", "instanceof", + "item", "layer", "left", "margin", "new", "opacity", "padding", "parent", "print", "rect", + "return", "right", "scale", "shaderInfo", "source", "sprite", "spriteSequence", "state", + "switch", "text", "this", "throw", "top", "try", "typeof", "var", "visible", "void", + "while", "with", "x", "y" + }; +}; + +} // namespace QmlDesigner diff --git a/src/plugins/qmldesigner/utils/uniquename.cpp b/src/plugins/qmldesigner/utils/uniquename.cpp deleted file mode 100644 index e77814c97cf..00000000000 --- a/src/plugins/qmldesigner/utils/uniquename.cpp +++ /dev/null @@ -1,93 +0,0 @@ -// Copyright (C) 2024 The Qt Company Ltd. -// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 - -#include "uniquename.h" - -#include -#include - -namespace QmlDesigner { - -/** - * @brief Generates a unique name based on the provided name. - * - * This method iteratively generates a name by appending suffixes until a unique name is found. - * The uniqueness of the generated name is determined by the provided predicate function. - * - * @param oldName The original name to be made unique. - * @param predicate A function that checks if a name is unique. Returns true if the name is unique, - * false otherwise. - * @return A unique name derived from the provided name. - */ -QString UniqueName::get(const QString &oldName, std::function predicate) -{ - QString newName = oldName; - while (!predicate(newName)) - newName = nextName(newName); - - return newName; -} - -/** - * @brief Generates a unique path based on the provided path. If the path belongs to a file, the - * filename or if it's a directory, the directory name will be adjusted to ensure uniqueness. - * - * This method appends a numerical suffix (or increment it if it exists) to the filename or - * directory name if necessary to make it unique. - * - * @param path The original path to be made unique. - * @return A unique path derived from the provided path. - */ -QString UniqueName::getPath(const QString &path) -{ - // Remove the trailing slash if it exists (otherwise QFileInfo::path() returns empty) - QString adjustedPath = path; - if (adjustedPath.endsWith('/')) - adjustedPath.chop(1); - - QFileInfo fileInfo = QFileInfo(adjustedPath); - QString baseName = fileInfo.baseName(); - QString suffix = fileInfo.completeSuffix(); - if (!suffix.isEmpty()) - suffix.prepend('.'); - - QString parentDir = fileInfo.path(); - QString pathTemplate = parentDir + "/%1" + suffix; - - QString uniqueBaseName = UniqueName::get(baseName, [&] (const QString &currName) { - return !QFileInfo::exists(pathTemplate.arg(currName)); - }); - - return pathTemplate.arg(uniqueBaseName); -} - -QString UniqueName::nextName(const QString &oldName) -{ - static QRegularExpression rgx("\\d+$"); // match a number at the end of a string - - QString uniqueName = oldName; - // if the name ends with a number, increment it - QRegularExpressionMatch match = rgx.match(uniqueName); - if (match.hasMatch()) { // ends with a number - QString numStr = match.captured(0); - int num = numStr.toInt() + 1; - - // get number of padding zeros, ex: for "005" = 2 - int nPaddingZeros = 0; - for (; nPaddingZeros < numStr.size() && numStr[nPaddingZeros] == '0'; ++nPaddingZeros); - - // if the incremented number's digits increased, decrease the padding zeros - if (std::fmod(std::log10(num), 1.0) == 0) - --nPaddingZeros; - - uniqueName = oldName.left(match.capturedStart()) - + QString('0').repeated(nPaddingZeros) - + QString::number(num); - } else { - uniqueName = oldName + '1'; - } - - return uniqueName; -} - -} // namespace QmlDesigner diff --git a/src/plugins/qmldesigner/utils/uniquename.h b/src/plugins/qmldesigner/utils/uniquename.h deleted file mode 100644 index 9c6765d1aee..00000000000 --- a/src/plugins/qmldesigner/utils/uniquename.h +++ /dev/null @@ -1,20 +0,0 @@ -// Copyright (C) 2024 The Qt Company Ltd. -// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 - -#pragma once - -#include - -namespace QmlDesigner { - -class UniqueName -{ -public: - static QString get(const QString &oldName, std::function predicate); - static QString getPath(const QString &oldName); - -private: - static QString nextName(const QString &oldName); -}; - -} // namespace QmlDesigner diff --git a/tests/unit/tests/testdesignercore/CMakeLists.txt b/tests/unit/tests/testdesignercore/CMakeLists.txt index 0bdf3452d6a..ebe7f7df120 100644 --- a/tests/unit/tests/testdesignercore/CMakeLists.txt +++ b/tests/unit/tests/testdesignercore/CMakeLists.txt @@ -147,6 +147,8 @@ add_qtc_library(TestDesignerCore OBJECT tracing/qmldesignertracing.cpp tracing/qmldesignertracing.h rewritertransaction.cpp rewritertransaction.h + uniquename.cpp + uniquename.h ) extend_qtc_library(TestDesignerCore