From 079d085a874d88e97849d1cb627d580c4e49b969 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Tue, 29 Sep 2026 07:32:12 +1300 Subject: [PATCH] Keep terminal uuids when a whole category is replaced; never duplicate one Review of the previous commit: - keep() could give an old uuid to a new terminal while another terminal of the new definition already carried it (a moved terminal), leaving two terminals with one uuid. A terminal carrying any old uuid is now left alone, and an old uuid already in use is never handed out. - copyDirectory() replaced a whole category of the embedded collection (drag a folder onto the project's folder of the same name) without carrying terminal uuids over. keepInDirectory() walks both trees by name and calls keep() on each symbol. - The "wire(s) not loaded" log line repeated the folio's list on every paste; it is now written only when a folio is loaded. tst_terminaluuids: 3 new cases, each red on the previous keep(). Co-Authored-By: Claude Opus 5.5 --- sources/ElementsCollection/terminaluuids.cpp | 65 +++++++++++++++++-- sources/ElementsCollection/terminaluuids.h | 2 + .../xmlelementcollection.cpp | 9 +++ sources/diagram.cpp | 2 +- tests/qttest/tst_terminaluuids.cpp | 62 ++++++++++++++++++ 5 files changed, 135 insertions(+), 5 deletions(-) diff --git a/sources/ElementsCollection/terminaluuids.cpp b/sources/ElementsCollection/terminaluuids.cpp index d19f831f8..ce45ec477 100644 --- a/sources/ElementsCollection/terminaluuids.cpp +++ b/sources/ElementsCollection/terminaluuids.cpp @@ -19,6 +19,7 @@ #include #include +#include #include namespace { @@ -64,26 +65,82 @@ QString terminalPlace(const QDomElement &terminal) one saved by a different version of the collection -- every wire on those symbols was lost the next time the project was opened. - Terminals that moved, and new ones, keep their own uuid. Where the old - definition has two terminals at one place, they are matched in order. + A terminal of the new definition that already carries one of the old + uuids is that same terminal, perhaps moved: it keeps it, and that uuid + is not given to anything else. Other terminals that moved, and new + ones, keep their own. No uuid is ever given to two terminals. Where the + old definition has two terminals at one place, they are matched in + order. */ void TerminalUuids::keep(const QDomElement &old_element, QDomElement &new_element) { QHash old_uuids; + QSet all_old_uuids; for (const QDomElement &t : terminalsOf(old_element)) { const QString uuid = t.attribute(QStringLiteral("uuid")); if (!uuid.isEmpty()) { old_uuids[terminalPlace(t)] << uuid; + all_old_uuids << uuid; } } if (old_uuids.isEmpty()) { return; } - for (QDomElement t : terminalsOf(new_element)) { + const QList new_terminals = terminalsOf(new_element); + QSet taken; + for (const QDomElement &t : new_terminals) { + taken << t.attribute(QStringLiteral("uuid")); + } + + for (QDomElement t : new_terminals) { + const QString own = t.attribute(QStringLiteral("uuid")); + if (all_old_uuids.contains(own)) { + continue; + } QStringList &uuids = old_uuids[terminalPlace(t)]; + while (!uuids.isEmpty() && taken.contains(uuids.first())) { + uuids.removeFirst(); + } if (!uuids.isEmpty()) { - t.setAttribute(QStringLiteral("uuid"), uuids.takeFirst()); + const QString uuid = uuids.takeFirst(); + taken.remove(own); + taken << uuid; + t.setAttribute(QStringLiteral("uuid"), uuid); + } + } +} + +/** + @brief TerminalUuids::keepInDirectory + keep() for every symbol of @p new_directory that has a counterpart of + the same name at the same place under @p old_directory. Both are + of an embedded collection; used when a whole category of + the project is replaced. +*/ +void TerminalUuids::keepInDirectory(const QDomElement &old_directory, + QDomElement &new_directory) +{ + for (QDomElement child = new_directory.firstChildElement(); + !child.isNull(); + child = child.nextSiblingElement()) { + if (child.tagName() != QLatin1String("category") + && child.tagName() != QLatin1String("element")) { + continue; + } + const QString name = child.attribute(QStringLiteral("name")); + QDomElement old_child = old_directory.firstChildElement(child.tagName()); + while (!old_child.isNull() + && old_child.attribute(QStringLiteral("name")) != name) { + old_child = old_child.nextSiblingElement(child.tagName()); + } + if (old_child.isNull()) { + continue; + } + if (child.tagName() == QLatin1String("category")) { + keepInDirectory(old_child, child); + } else { + keep(old_child, child); } } } diff --git a/sources/ElementsCollection/terminaluuids.h b/sources/ElementsCollection/terminaluuids.h index 872acbdd7..f22c7dbec 100644 --- a/sources/ElementsCollection/terminaluuids.h +++ b/sources/ElementsCollection/terminaluuids.h @@ -23,6 +23,8 @@ class QDomElement; namespace TerminalUuids { void keep(const QDomElement &old_element, QDomElement &new_element); + void keepInDirectory(const QDomElement &old_directory, + QDomElement &new_directory); } #endif // TERMINALUUIDS_H diff --git a/sources/ElementsCollection/xmlelementcollection.cpp b/sources/ElementsCollection/xmlelementcollection.cpp index ee6a8b1be..ebd64f886 100644 --- a/sources/ElementsCollection/xmlelementcollection.cpp +++ b/sources/ElementsCollection/xmlelementcollection.cpp @@ -870,6 +870,15 @@ ElementsLocation XmlElementCollection::copyDirectory( created_location.setPath(destination.projectCollectionPath() % "/" % new_dir_name); } + //The symbols of the replaced directory keep their terminal uuids, + //see TerminalUuids::keep() + if (!element.isNull()) { + QDomElement new_dir_dom = directory(created_location.collectionPath(false)); + if (!new_dir_dom.isNull()) { + TerminalUuids::keepInDirectory(element, new_dir_dom); + } + } + emit directorieAdded(created_location.collectionPath(false)); return created_location; } diff --git a/sources/diagram.cpp b/sources/diagram.cpp index 9ffaede29..332f083b6 100644 --- a/sources/diagram.cpp +++ b/sources/diagram.cpp @@ -1925,7 +1925,7 @@ bool Diagram::fromXml(QDomElement &document, delete c; } } - if (!m_wires_not_reconnected.isEmpty()) { + if (consider_informations && !m_wires_not_reconnected.isEmpty()) { qWarning().noquote() << "Diagram::fromXml():" << m_wires_not_reconnected.size() << "wire(s) not loaded, a terminal they join was not found:" diff --git a/tests/qttest/tst_terminaluuids.cpp b/tests/qttest/tst_terminaluuids.cpp index e641f3c57..9da458228 100644 --- a/tests/qttest/tst_terminaluuids.cpp +++ b/tests/qttest/tst_terminaluuids.cpp @@ -159,6 +159,68 @@ private slots: QCOMPARE(uuids(new_symbol), (QStringList{"{a}", "{b}", "{z}"})); } + // The new revision moved terminal {a} and put a new one where it was: + // {a} is still {a}, and the new one is not given {a} a second time. + void noUuidGivenTwice() + { + QDomDocument doc; + const QDomElement old_symbol = symbol(doc, {{"0", "0", "n", "{a}"}}); + QDomElement new_symbol = symbol(doc, {{"0", "5", "n", "{a}"}, + {"0", "0", "n", "{c}"}}); + TerminalUuids::keep(old_symbol, new_symbol); + QCOMPARE(uuids(new_symbol), (QStringList{"{a}", "{c}"})); + } + + // A terminal that already carries an old uuid is never renamed, even + // when it moved onto the place of another old terminal. + void movedTerminalNotRenamed() + { + QDomDocument doc; + const QDomElement old_symbol = symbol(doc, {{"0", "0", "n", "{a}"}, + {"0", "5", "n", "{b}"}}); + QDomElement new_symbol = symbol(doc, {{"0", "5", "n", "{a}"}, + {"0", "9", "n", "{x}"}}); + TerminalUuids::keep(old_symbol, new_symbol); + QCOMPARE(uuids(new_symbol), (QStringList{"{a}", "{x}"})); + } + + // Replacing a whole category: every symbol with a counterpart of the + // same name, at any depth, keeps its terminal uuids. + void keepInDirectory() + { + QDomDocument doc; + auto category = [&doc](const QString &name) { + QDomElement c = doc.createElement(QStringLiteral("category")); + c.setAttribute(QStringLiteral("name"), name); + return c; + }; + auto named = [](QDomElement e, const QString &name) { + e.setAttribute(QStringLiteral("name"), name); + return e; + }; + + QDomElement old_dir = category(QStringLiteral("dir")); + QDomElement old_sub = category(QStringLiteral("sub")); + old_dir.appendChild(named(symbol(doc, {{"0", "0", "n", "{a}"}}), "top.elmt")); + old_dir.appendChild(old_sub); + old_sub.appendChild(named(symbol(doc, {{"0", "0", "n", "{b}"}}), "deep.elmt")); + + QDomElement new_dir = category(QStringLiteral("dir")); + QDomElement new_sub = category(QStringLiteral("sub")); + const QDomElement top = named(symbol(doc, {{"0", "0", "n", "{x}"}}), "top.elmt"); + const QDomElement other = named(symbol(doc, {{"0", "0", "n", "{y}"}}), "other.elmt"); + const QDomElement deep = named(symbol(doc, {{"0", "0", "n", "{z}"}}), "deep.elmt"); + new_dir.appendChild(top); + new_dir.appendChild(other); + new_dir.appendChild(new_sub); + new_sub.appendChild(deep); + + TerminalUuids::keepInDirectory(old_dir, new_dir); + QCOMPARE(uuids(top), QStringList{"{a}"}); + QCOMPARE(uuids(other), QStringList{"{y}"}); + QCOMPARE(uuids(deep), QStringList{"{b}"}); + } + // The real loader, on a project whose 131 wires are saved against // terminal uuids: replaced symbols lose 119 of them unless keep() ran, // and the ones lost are reported.