From e6ac117f9bb997be42d90e9db85756d201e8dc32 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Tue, 29 Sep 2026 07:21:19 +1300 Subject: [PATCH 1/2] Keep wires attached when a project's copy of a symbol is replaced A wire whose terminals have uuids is saved against them, and on load it is reattached to a terminal with that uuid or dropped, with only a qDebug line. Re-importing a changed symbol and choosing "replace" swapped the project's definition for one whose terminal uuids differ; the placed symbols kept the old ones until the project was reopened, so every wire on them was lost at the next open, silently, and gone for good at the next save. - XmlElementCollection::copyElement(), where an embedded definition is overwritten, carries each old terminal uuid onto the new terminal at the same place and orientation (TerminalUuids::keep()). Terminals that moved, and new ones, keep their own. - Diagram::fromXml() records wires it could not reattach, logs them, and the editor lists them in one warning after opening a project. Measured on 2612_ats_singlephase.qet with the stored splice's terminal uuids made to differ from the collection's: replace, save, reopen loads 34 of 131 wires on master, 131 with this change (GUI, both arms). tst_terminaluuids covers keep() and runs the real loader. Co-Authored-By: Claude Opus 5.5 --- cmake/qet_compilation_vars.cmake | 2 + sources/ElementsCollection/terminaluuids.cpp | 89 +++++++++ sources/ElementsCollection/terminaluuids.h | 28 +++ .../xmlelementcollection.cpp | 2 + sources/diagram.cpp | 47 +++++ sources/diagram.h | 6 +- sources/qetdiagrameditor.cpp | 28 +++ tests/qttest/CMakeLists.txt | 14 ++ tests/qttest/tst_terminaluuids.cpp | 186 ++++++++++++++++++ 9 files changed, 401 insertions(+), 1 deletion(-) create mode 100644 sources/ElementsCollection/terminaluuids.cpp create mode 100644 sources/ElementsCollection/terminaluuids.h create mode 100644 tests/qttest/tst_terminaluuids.cpp diff --git a/cmake/qet_compilation_vars.cmake b/cmake/qet_compilation_vars.cmake index f46101bfa..5b3f13eb7 100644 --- a/cmake/qet_compilation_vars.cmake +++ b/cmake/qet_compilation_vars.cmake @@ -475,6 +475,8 @@ set(QET_SRC_FILES ${QET_DIR}/sources/ElementsCollection/elementstreeview.h ${QET_DIR}/sources/ElementsCollection/fileelementcollectionitem.cpp ${QET_DIR}/sources/ElementsCollection/fileelementcollectionitem.h + ${QET_DIR}/sources/ElementsCollection/terminaluuids.cpp + ${QET_DIR}/sources/ElementsCollection/terminaluuids.h ${QET_DIR}/sources/ElementsCollection/xmlelementcollection.cpp ${QET_DIR}/sources/ElementsCollection/xmlelementcollection.h ${QET_DIR}/sources/ElementsCollection/xmlprojectelementcollectionitem.cpp diff --git a/sources/ElementsCollection/terminaluuids.cpp b/sources/ElementsCollection/terminaluuids.cpp new file mode 100644 index 000000000..d19f831f8 --- /dev/null +++ b/sources/ElementsCollection/terminaluuids.cpp @@ -0,0 +1,89 @@ +/* + Copyright 2006-2026 The QElectroTech Team + This file is part of QElectroTech. + + QElectroTech is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 2 of the License, or + (at your option) any later version. + + QElectroTech is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with QElectroTech. If not, see . +*/ +#include "terminaluuids.h" + +#include +#include +#include + +namespace { +/** + @return the of the definition held by the embedded + collection element @p collection_element +*/ +QList terminalsOf(const QDomElement &collection_element) +{ + QList terminals; + const QDomElement description = collection_element + .firstChildElement(QStringLiteral("definition")) + .firstChildElement(QStringLiteral("description")); + for (QDomElement t = description.firstChildElement(QStringLiteral("terminal")); + !t.isNull(); + t = t.nextSiblingElement(QStringLiteral("terminal"))) { + terminals << t; + } + return terminals; +} + + //Place and orientation, "10" and "10.0" being the same place +QString terminalPlace(const QDomElement &terminal) +{ + return QStringLiteral("%1|%2|%3") + .arg(QString::number(terminal.attribute(QStringLiteral("x")).toDouble()), + QString::number(terminal.attribute(QStringLiteral("y")).toDouble()), + terminal.attribute(QStringLiteral("orientation"))); +} +} + +/** + @brief TerminalUuids::keep + Give each terminal of @p new_element the uuid of the terminal of + @p old_element at the same place and orientation. Both are + of an embedded collection. + + A wire is saved against the uuids of the terminals it joins, and on + loading it is reattached to a terminal with that uuid or not at all. + The symbols already on the folios keep the terminals of the definition + they were built from, so when the project's definition is replaced by + one whose terminal uuids differ -- another copy of the same symbol, or + 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. +*/ +void TerminalUuids::keep(const QDomElement &old_element, QDomElement &new_element) +{ + QHash old_uuids; + for (const QDomElement &t : terminalsOf(old_element)) { + const QString uuid = t.attribute(QStringLiteral("uuid")); + if (!uuid.isEmpty()) { + old_uuids[terminalPlace(t)] << uuid; + } + } + if (old_uuids.isEmpty()) { + return; + } + + for (QDomElement t : terminalsOf(new_element)) { + QStringList &uuids = old_uuids[terminalPlace(t)]; + if (!uuids.isEmpty()) { + t.setAttribute(QStringLiteral("uuid"), uuids.takeFirst()); + } + } +} diff --git a/sources/ElementsCollection/terminaluuids.h b/sources/ElementsCollection/terminaluuids.h new file mode 100644 index 000000000..872acbdd7 --- /dev/null +++ b/sources/ElementsCollection/terminaluuids.h @@ -0,0 +1,28 @@ +/* + Copyright 2006-2026 The QElectroTech Team + This file is part of QElectroTech. + + QElectroTech is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 2 of the License, or + (at your option) any later version. + + QElectroTech is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with QElectroTech. If not, see . +*/ +#ifndef TERMINALUUIDS_H +#define TERMINALUUIDS_H + +class QDomElement; + +namespace TerminalUuids +{ + void keep(const QDomElement &old_element, QDomElement &new_element); +} + +#endif // TERMINALUUIDS_H diff --git a/sources/ElementsCollection/xmlelementcollection.cpp b/sources/ElementsCollection/xmlelementcollection.cpp index b124b72ff..ee6a8b1be 100644 --- a/sources/ElementsCollection/xmlelementcollection.cpp +++ b/sources/ElementsCollection/xmlelementcollection.cpp @@ -21,6 +21,7 @@ #include "../qetproject.h" #include "../qetxml.h" #include "elementslocation.h" +#include "terminaluuids.h" /** @brief XmlElementCollection::XmlElementCollection @@ -914,6 +915,7 @@ ElementsLocation XmlElementCollection::copyElement( % "/" % new_elmt_name); bool removed = false; if (!element.isNull()) { + TerminalUuids::keep(element, elmt_dom); element.parentNode().removeChild(element); removed = true; } diff --git a/sources/diagram.cpp b/sources/diagram.cpp index 008fa9495..9ffaede29 100644 --- a/sources/diagram.cpp +++ b/sources/diagram.cpp @@ -773,6 +773,18 @@ QUuid Diagram::uuid() return m_uuid; } +/** + @brief Diagram::wiresNotReconnected + @return one line per wire of the loaded file that was left out because + a terminal it joins could not be found, for example after the symbol's + definition in the project was replaced by one whose terminals differ. + Empty for a folio that loaded every wire. +*/ +QStringList Diagram::wiresNotReconnected() const +{ + return m_wires_not_reconnected; +} + /** @brief Diagram::uuidUsedByOtherDiagram A hand-edited or merged project file can contain two folios with the same @@ -1837,6 +1849,8 @@ bool Diagram::fromXml(QDomElement &document, } // Load conductor + if (consider_informations) + m_wires_not_reconnected.clear(); QList added_conductors; for (auto f : QET::findInDomElement(root, QStringLiteral("conductors"), @@ -1849,6 +1863,33 @@ bool Diagram::fromXml(QDomElement &document, Terminal* p1 = findTerminal(1, f, table_adr_id, added_elements); Terminal* p2 = findTerminal(2, f, table_adr_id, added_elements); + //Keep a trace of the wire, it will be missing from the next save + if ((!p1 || !p2) && consider_informations) + { + //The symbol's label, else its name. For an end not found, + //only the uuid form of a wire says which symbol it is on. + auto end_label = [&f, &added_elements](const QString &index, + Terminal *found) { + Element *element = found ? found->parentElement() : nullptr; + const QUuid uuid(f.attribute(QStringLiteral("element") + index)); + for (int i = 0 ; !element && !uuid.isNull() + && i < added_elements.size() ; ++i) { + if (added_elements.at(i)->uuid() == uuid) + element = added_elements.at(i); + } + if (!element) + return QStringLiteral("?"); + const QString label = element->actualLabel(); + return label.isEmpty() ? element->name() : label; + }; + QString wire = QStringLiteral("%1 - %2").arg(end_label(QStringLiteral("1"), p1), + end_label(QStringLiteral("2"), p2)); + const QString num = f.attribute(QStringLiteral("num")); + if (!num.isEmpty() && num != QLatin1String("_")) + wire += QStringLiteral(" (%1)").arg(num); + m_wires_not_reconnected << wire; + } + if (p1 && p2 && p1 != p2) { Conductor *c = new Conductor(p1, p2); @@ -1884,6 +1925,12 @@ bool Diagram::fromXml(QDomElement &document, delete c; } } + if (!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:" + << m_wires_not_reconnected.join(QStringLiteral(", ")); + } //Filling of falculatory lists if (content_ptr) { diff --git a/sources/diagram.h b/sources/diagram.h index 83d408835..96299d036 100644 --- a/sources/diagram.h +++ b/sources/diagram.h @@ -148,7 +148,10 @@ class Diagram : public QGraphicsScene bool uuidUsedByOtherDiagram(const QUuid &uuid) const; QUuid derivedUuid(const QDomElement &root, const QString &reason) const; - + + //Wires of the loaded file whose ends could not be found + QStringList m_wires_not_reconnected; + // METHODS protected: void drawBackground(QPainter *, const QRectF &) override; @@ -171,6 +174,7 @@ class Diagram : public QGraphicsScene void correctTextPos(Element* elmt); void restoreText(Element* elmt); QUuid uuid(); + QStringList wiresNotReconnected() const; void setEventInterface (DiagramEventInterface *event_interface); void clearEventInterface(); diff --git a/sources/qetdiagrameditor.cpp b/sources/qetdiagrameditor.cpp index 80d52f4dd..7cb805bc3 100644 --- a/sources/qetdiagrameditor.cpp +++ b/sources/qetdiagrameditor.cpp @@ -1672,6 +1672,34 @@ bool QETDiagramEditor::openAndAddProject( ); } + //Report wires left out of the load because a terminal they join + //was not found: they would otherwise vanish on the next save + //without the user ever being told. + QStringList lost_wires; + for (Diagram *diagram : project->diagrams()) { + for (const QString &wire : diagram->wiresNotReconnected()) { + lost_wires << tr("Folio %1 : %2").arg(diagram->folioIndex() + 1).arg(wire); + } + } + if (interactive && !lost_wires.isEmpty()) + { + QMessageBox box(QMessageBox::Warning, + tr("Conducteurs non chargés", "message box title"), + tr("%n conducteur(s) n'ont pas pu être reliés à leurs" + " bornes et n'ont pas été chargés. La définition de" + " l'élément dans le projet a probablement été remplacée" + " par une autre dont les bornes diffèrent.\n\n" + "Si vous enregistrez le projet, ces conducteurs" + " disparaîtront du fichier. Fermez-le sans enregistrer pour" + " conserver le fichier tel quel.", + "message box content", + lost_wires.size()), + QMessageBox::Ok, + this); + box.setDetailedText(lost_wires.join(QLatin1Char('\n'))); + box.exec(); + } + BackupDialog backup_dialog(this); if (backup_dialog.exec() == QDialog::Accepted) { diff --git a/tests/qttest/CMakeLists.txt b/tests/qttest/CMakeLists.txt index e2fde811c..9656c557f 100644 --- a/tests/qttest/CMakeLists.txt +++ b/tests/qttest/CMakeLists.txt @@ -385,6 +385,20 @@ target_compile_definitions(tst_resaveunchanged PRIVATE "QET_TEST_BINARY_PATH=\"$\"" "QET_EXAMPLES_DIR=\"${QET_DIR}/examples\"") +# Replacing a symbol's definition in a project keeps its terminal uuids, so +# the wires saved against them still load: TerminalUuids::keep() on its own, +# then the real binary's --info on an example whose symbols were replaced. +add_executable( + tst_terminaluuids + tst_terminaluuids.cpp + ${QET_DIR}/sources/ElementsCollection/terminaluuids.cpp) +add_test(NAME tst_terminaluuids COMMAND tst_terminaluuids) +add_dependencies(tst_terminaluuids qelectrotech) +target_link_libraries(tst_terminaluuids PRIVATE Qt::Test Qt::Xml) +target_compile_definitions(tst_terminaluuids PRIVATE + "QET_TEST_BINARY_PATH=\"$\"" + "QET_EXAMPLES_DIR=\"${QET_DIR}/examples\"") + # DiagramContext::fromXml() -- the two readers (QDom for projects, pugixml # for element definitions in the collection) give the same values: stray # spaces trimmed, accents kept. diff --git a/tests/qttest/tst_terminaluuids.cpp b/tests/qttest/tst_terminaluuids.cpp new file mode 100644 index 000000000..e641f3c57 --- /dev/null +++ b/tests/qttest/tst_terminaluuids.cpp @@ -0,0 +1,186 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +#include "../../sources/ElementsCollection/terminaluuids.h" + +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +// Replacing a symbol's definition in a project must not cost the wires on +// it. A wire is saved against the uuids of the terminals it joins and is +// reattached on load to a terminal with that uuid, or not at all; the +// replacement is another copy of the symbol, whose terminals usually carry +// other uuids. TerminalUuids::keep() carries the old uuids over. +namespace { + +QDomElement symbol(QDomDocument &doc, const QList &terminals) +{ + QDomElement element = doc.createElement(QStringLiteral("element")); + QDomElement definition = doc.createElement(QStringLiteral("definition")); + QDomElement description = doc.createElement(QStringLiteral("description")); + element.appendChild(definition); + definition.appendChild(description); + for (const QStringList &t : terminals) { + QDomElement terminal = doc.createElement(QStringLiteral("terminal")); + terminal.setAttribute(QStringLiteral("x"), t.at(0)); + terminal.setAttribute(QStringLiteral("y"), t.at(1)); + terminal.setAttribute(QStringLiteral("orientation"), t.at(2)); + if (t.size() > 3) + terminal.setAttribute(QStringLiteral("uuid"), t.at(3)); + description.appendChild(terminal); + } + return element; +} + +QStringList uuids(const QDomElement &element) +{ + QStringList out; + const QDomNodeList nodes = element.elementsByTagName(QStringLiteral("terminal")); + for (int i = 0; i < nodes.size(); ++i) + out << nodes.at(i).toElement().attribute(QStringLiteral("uuid")); + return out; +} + +QList embeddedSymbols(const QDomDocument &doc) +{ + QList out; + const QDomNodeList nodes = doc.documentElement() + .firstChildElement(QStringLiteral("collection")) + .elementsByTagName(QStringLiteral("element")); + for (int i = 0; i < nodes.size(); ++i) + out << nodes.at(i).toElement(); + return out; +} + +} + +class tst_terminaluuids : public QObject +{ + Q_OBJECT + + QTemporaryDir m_dir; + int m_run = 0; + + // --info on @p project in a sandbox of its own; returns the number of + // wires loaded and puts what was written on stderr in @p log. + int loadedWires(const QString &project, QString *log) + { + const QString home = m_dir.filePath(QStringLiteral("home%1").arg(m_run++)); + QDir().mkpath(home); + QProcessEnvironment env = QProcessEnvironment::systemEnvironment(); + env.insert(QStringLiteral("QT_QPA_PLATFORM"), QStringLiteral("offscreen")); + env.insert(QStringLiteral("HOME"), home); + env.insert(QStringLiteral("XDG_CONFIG_HOME"), home + QStringLiteral("/config")); + env.insert(QStringLiteral("XDG_DATA_HOME"), home + QStringLiteral("/data")); + QProcess proc; + proc.setProcessEnvironment(env); + proc.start(QStringLiteral(QET_TEST_BINARY_PATH), {QStringLiteral("--info"), project}); + if (!proc.waitForFinished(120000) || proc.exitCode() != 0) + return -1; + *log = QString::fromUtf8(proc.readAllStandardError()); + const QByteArray out = proc.readAllStandardOutput(); + const QJsonDocument json = QJsonDocument::fromJson(out.mid(out.indexOf('{'))); + return json.object().value(QStringLiteral("conductors")).toInt(-1); + } + + // 2612_ats_singlephase.qet with each embedded symbol replaced by a copy + // whose terminals carry new uuids, passed through keep() or not. + QString replacedSymbols(bool keep_uuids) + { + QFile in(QStringLiteral(QET_EXAMPLES_DIR "/2612_ats_singlephase.qet")); + if (!in.open(QIODevice::ReadOnly)) return {}; + QDomDocument doc; + if (!doc.setContent(&in)) return {}; + for (const QDomElement &old_symbol : embeddedSymbols(doc)) { + QDomElement new_symbol = old_symbol.cloneNode().toElement(); + const QDomNodeList terminals = new_symbol.elementsByTagName(QStringLiteral("terminal")); + for (int i = 0; i < terminals.size(); ++i) + terminals.at(i).toElement().setAttribute(QStringLiteral("uuid"), + QUuid::createUuid().toString()); + if (keep_uuids) + TerminalUuids::keep(old_symbol, new_symbol); + old_symbol.parentNode().replaceChild(new_symbol, old_symbol); + } + const QString path = m_dir.filePath(keep_uuids ? QStringLiteral("kept.qet") + : QStringLiteral("lost.qet")); + QFile out(path); + if (!out.open(QIODevice::WriteOnly)) return {}; + out.write(doc.toByteArray()); + return path; + } + +private slots: + void samePlaceTakesOldUuid() + { + QDomDocument doc; + const QDomElement old_symbol = symbol(doc, {{"0", "10", "n", "{a}"}, + {"20", "10", "s", "{b}"}}); + QDomElement new_symbol = symbol(doc, {{"20.0", "10", "s", "{y}"}, + {"0", "10.0", "n", "{x}"}}); + TerminalUuids::keep(old_symbol, new_symbol); + QCOMPARE(uuids(new_symbol), (QStringList{"{b}", "{a}"})); + } + + void movedOrNewTerminalKeepsItsOwn() + { + QDomDocument doc; + const QDomElement old_symbol = symbol(doc, {{"0", "10", "n", "{a}"}}); + QDomElement new_symbol = symbol(doc, {{"0", "10", "e", "{x}"}, + {"0", "20", "n", "{y}"}, + {"0", "30", "n"}}); + TerminalUuids::keep(old_symbol, new_symbol); + QCOMPARE(uuids(new_symbol), (QStringList{"{x}", "{y}", ""})); + } + + void oldWithoutUuidChangesNothing() + { + QDomDocument doc; + const QDomElement old_symbol = symbol(doc, {{"0", "10", "n"}}); + QDomElement new_symbol = symbol(doc, {{"0", "10", "n", "{x}"}}); + TerminalUuids::keep(old_symbol, new_symbol); + QCOMPARE(uuids(new_symbol), (QStringList{"{x}"})); + } + + void twoAtOnePlaceMatchedInOrder() + { + QDomDocument doc; + const QDomElement old_symbol = symbol(doc, {{"0", "10", "n", "{a}"}, + {"0", "10", "n", "{b}"}}); + QDomElement new_symbol = symbol(doc, {{"0", "10", "n", "{x}"}, + {"0", "10", "n", "{y}"}, + {"0", "10", "n", "{z}"}}); + TerminalUuids::keep(old_symbol, new_symbol); + QCOMPARE(uuids(new_symbol), (QStringList{"{a}", "{b}", "{z}"})); + } + + // 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. + void wiresSurviveReplacedSymbols() + { + QVERIFY(m_dir.isValid()); + QString log; + + const QString lost = replacedSymbols(false); + QVERIFY(!lost.isEmpty()); + QCOMPARE(loadedWires(lost, &log), 12); + QVERIFY2(log.contains(QStringLiteral("55 wire(s) not loaded")) + && log.contains(QStringLiteral("64 wire(s) not loaded")), + "the lost wires were not reported"); + + const QString kept = replacedSymbols(true); + QVERIFY(!kept.isEmpty()); + QCOMPARE(loadedWires(kept, &log), 131); + QVERIFY2(!log.contains(QStringLiteral("not loaded")), "a wire was reported lost"); + } +}; + +QTEST_APPLESS_MAIN(tst_terminaluuids) + +#include "tst_terminaluuids.moc" From 079d085a874d88e97849d1cb627d580c4e49b969 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Tue, 29 Sep 2026 07:32:12 +1300 Subject: [PATCH 2/2] 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.