From 0721b42e2103705dcb2d1983fa14dad942204074 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Mon, 28 Sep 2026 20:13:31 +1300 Subject: [PATCH 1/7] Give symbols saved without a uuid the same one on every load A symbol saved without a uuid got a random one from Element::fromXml() on every load, and the next save wrote it out: two loads of the same file gave the same symbol two identities, and anything pointing at it by uuid (a script, a comparison of two versions, a wire's identity) could not follow it from one session to the next. When a folio is loaded, such a symbol now gets a UUID v5 derived from what it is and where it sits: its type, its position on the folio and its orientation. Never the folio's index, so inserting or moving a folio does not change it. Identical symbols stacked on one spot, or a copied folio, are told apart by a counter kept per project (QETProject::derivedUuid()), in load order among those symbols alone. A paste still renews uuids. Symbols that have a uuid in the file keep it. All 24 example projects already have one for every symbol, so they are unchanged; with the symbols' uuids stripped, each saves byte-for-byte the same twice (master: different every time). tst_derivedsymboluuid runs --resave on a fixture with its uuids stripped: same uuids on every load, same after a folio is inserted in front, saved uuids kept, stacked copies differ. The first two fail without this change. Co-Authored-By: Claude Opus 5.5 --- sources/diagram.cpp | 17 +++ sources/qetgraphicsitem/element.h | 1 + sources/qetproject.cpp | 21 +++ sources/qetproject.h | 2 + tests/qttest/CMakeLists.txt | 12 ++ tests/qttest/tst_derivedsymboluuid.cpp | 184 +++++++++++++++++++++++++ 6 files changed, 237 insertions(+) create mode 100644 tests/qttest/tst_derivedsymboluuid.cpp diff --git a/sources/diagram.cpp b/sources/diagram.cpp index 4831b6484..bc0cd7c9e 100644 --- a/sources/diagram.cpp +++ b/sources/diagram.cpp @@ -1676,6 +1676,23 @@ bool Diagram::fromXml(QDomElement &document, delete nvel_elmt; qDebug() << QStringLiteral("Diagram::fromXml() : Le chargement des parametres d'un element a echoue"); } else { + //A symbol saved without a uuid got a random one from + //Element::fromXml(): a different identity on every load, + //written out on the next save. Derive it instead from what + //the symbol is and where it sits on its folio -- never from + //the folio's index, so inserting or moving a folio does not + //change it. Only for a folio being loaded: a paste renews + //uuids anyway. + if (consider_informations && m_project + && QUuid(element_xml.attribute(QStringLiteral("uuid"))).isNull()) { + nvel_elmt->setUuid(m_project->derivedUuid( + QStringLiteral("element"), + QStringList{type_id, + element_xml.attribute(QStringLiteral("x")), + element_xml.attribute(QStringLiteral("y")), + element_xml.attribute(QStringLiteral("orientation"))} + .join(QLatin1Char('\n')))); + } ItemGroups::setGroup(nvel_elmt, ItemGroups::read(element_xml)); added_elements << nvel_elmt; } diff --git a/sources/qetgraphicsitem/element.h b/sources/qetgraphicsitem/element.h index 9bb02202d..33d8b752e 100644 --- a/sources/qetgraphicsitem/element.h +++ b/sources/qetgraphicsitem/element.h @@ -245,6 +245,7 @@ class Element : public QetGraphicsItem QString linkTypeToString() const; void newUuid() {m_uuid = QUuid::createUuid();} //create new uuid for this element + void setUuid(const QUuid &uuid) {m_uuid = uuid;} protected: void drawAxes(QPainter *, const QStyleOptionGraphicsItem *); diff --git a/sources/qetproject.cpp b/sources/qetproject.cpp index df45c2c08..e66714577 100644 --- a/sources/qetproject.cpp +++ b/sources/qetproject.cpp @@ -275,6 +275,27 @@ QUuid QETProject::uuid() const return m_uuid; } +/** + @brief QETProject::derivedUuid + A uuid for an item of this project that was saved without one, the same + on every load of the same file. + @p key describes the item by what it is, never by its place in the file + or its folio's index: inserting or moving a folio must not change it. + Items with the same @p kind and @p key anywhere in the project (a copied + folio, two identical symbols stacked on one spot) are told apart by a + counter, in load order among those items alone. + @return a UUID v5, which cannot collide with the v4 uuids given to new + items +*/ +QUuid QETProject::derivedUuid(const QString &kind, const QString &key) +{ + static const QUuid derived_ns(QStringLiteral("{7d1e9c3a-5b2f-4e8a-9c61-2f4b8d0e6a17}")); + const QString full = kind + QLatin1Char('\n') + key; + const int n = m_derived_uuid_keys[full]++; + return QUuid::createUuidV5(derived_ns, + n ? full + QLatin1Char('\n') + QString::number(n) : full); +} + /** @brief QETProject::init */ diff --git a/sources/qetproject.h b/sources/qetproject.h index e05f3b216..991b9f1a9 100644 --- a/sources/qetproject.h +++ b/sources/qetproject.h @@ -107,6 +107,7 @@ class QETProject : public QObject ProjectPropertiesHandler& projectPropertiesHandler(); projectDataBase *dataBase(); QUuid uuid() const; + QUuid derivedUuid(const QString &kind, const QString &key); ProjectState state() const; QList diagrams() const; int folioIndex(const Diagram *) const; @@ -365,6 +366,7 @@ class QETProject : public QObject QFuture m_backup_future; KAutoSaveFile m_backup_file; QUuid m_uuid = QUuid::createUuid(); + QHash m_derived_uuid_keys; projectDataBase m_data_base; QVector m_terminal_strip_vector; diff --git a/tests/qttest/CMakeLists.txt b/tests/qttest/CMakeLists.txt index 94283cc09..52d156547 100644 --- a/tests/qttest/CMakeLists.txt +++ b/tests/qttest/CMakeLists.txt @@ -328,3 +328,15 @@ target_include_directories(tst_conductorselfretrace PRIVATE ${QET_DIR}/sources) target_link_libraries(tst_conductorselfretrace PRIVATE Qt::Test) target_compile_definitions(tst_conductorselfretrace PRIVATE "QET_TEST_BINARY_PATH=\"$\"") + +# A symbol saved without a uuid gets the same one on every load, and +# inserting a folio does not change it. Runs the real binary's --resave on +# fixtures/qet_bug_repro_resaved.qet with the symbols' uuids stripped. +add_executable( + tst_derivedsymboluuid + tst_derivedsymboluuid.cpp) +add_test(NAME tst_derivedsymboluuid COMMAND tst_derivedsymboluuid) +add_dependencies(tst_derivedsymboluuid qelectrotech) +target_link_libraries(tst_derivedsymboluuid PRIVATE Qt::Test Qt::Xml) +target_compile_definitions(tst_derivedsymboluuid PRIVATE + "QET_TEST_BINARY_PATH=\"$\"") diff --git a/tests/qttest/tst_derivedsymboluuid.cpp b/tests/qttest/tst_derivedsymboluuid.cpp new file mode 100644 index 000000000..d3fa5861a --- /dev/null +++ b/tests/qttest/tst_derivedsymboluuid.cpp @@ -0,0 +1,184 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +// A symbol saved without a uuid must get the same one on every load of the +// same file, and inserting a folio in front of it must not change it. Until +// this was fixed it got a random uuid each time (Element::fromXml), which +// the next save wrote out. +// +// Runs the real binary (--resave) on the fixture with its symbols' uuids +// stripped, and reads the uuids back from the saved file. +namespace { + +QDomDocument load(const QString &path) +{ + QDomDocument doc; + QFile file(path); + if (file.open(QIODevice::ReadOnly)) + doc.setContent(&file); + return doc; +} + +QList symbols(const QDomElement &diagram) +{ + QList out; + const QDomNodeList nodes = diagram.firstChildElement(QStringLiteral("elements")) + .elementsByTagName(QStringLiteral("element")); + for (int i = 0; i < nodes.size(); ++i) { + const QDomElement e = nodes.at(i).toElement(); + if (e.parentNode().parentNode() == diagram) + out << e; + } + return out; +} + +QList diagrams(const QDomDocument &doc) +{ + QList out; + for (QDomElement d = doc.documentElement().firstChildElement(QStringLiteral("diagram")); + !d.isNull(); d = d.nextSiblingElement(QStringLiteral("diagram"))) + out << d; + return out; +} + +// Symbols without uuids, and no wires (they refer to symbols by uuid). +void stripUuids(QDomDocument &doc) +{ + for (QDomElement d : diagrams(doc)) { + for (QDomElement e : symbols(d)) + e.removeAttribute(QStringLiteral("uuid")); + d.removeChild(d.firstChildElement(QStringLiteral("conductors"))); + } +} + +// type|x|y|orientation -> uuid, for every symbol in the file +QMultiHash symbolUuids(const QDomDocument &doc) +{ + QMultiHash out; + for (const QDomElement &d : diagrams(doc)) + for (const QDomElement &e : symbols(d)) + out.insert(QStringList{e.attribute(QStringLiteral("type")), + e.attribute(QStringLiteral("x")), + e.attribute(QStringLiteral("y")), + e.attribute(QStringLiteral("orientation"))} + .join(QLatin1Char('|')), + e.attribute(QStringLiteral("uuid"))); + return out; +} + +} // namespace + +class tst_derivedsymboluuid : public QObject +{ + Q_OBJECT + + QTemporaryDir m_dir; + int m_run = 0; + + // Save @p doc, run --resave on it in a sandbox of its own (so a running + // QElectroTech cannot answer instead), and return what was saved. + QDomDocument resave(const QDomDocument &doc) + { + const QString in = m_dir.filePath(QStringLiteral("in%1.qet").arg(m_run)); + const QString out = m_dir.filePath(QStringLiteral("out%1.qet").arg(m_run)); + const QString home = m_dir.filePath(QStringLiteral("home%1").arg(m_run++)); + QDir().mkpath(home); + QFile f(in); + if (!f.open(QIODevice::WriteOnly)) return {}; + f.write(doc.toByteArray()); + f.close(); + + 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("--resave"), in, out}); + if (!proc.waitForFinished(60000) || proc.exitCode() != 0) return {}; + return load(out); + } + + QDomDocument fixture() + { + QDomDocument doc = load(QFINDTESTDATA("fixtures/qet_bug_repro_resaved.qet")); + stripUuids(doc); + return doc; + } + +private slots: + void initTestCase() + { + QVERIFY(m_dir.isValid()); + QVERIFY(QFile::exists(QStringLiteral(QET_TEST_BINARY_PATH))); + QVERIFY(!fixture().isNull()); + } + + void sameUuidsOnEveryLoad() + { + const QMultiHash a = symbolUuids(resave(fixture())); + const QMultiHash b = symbolUuids(resave(fixture())); + QCOMPARE(a.size(), 9); // the fixture's placed symbols + QCOMPARE(a, b); + for (const QString &u : a) + QVERIFY2(!QUuid(u).isNull(), qPrintable(u)); + QCOMPARE(QSet(a.begin(), a.end()).size(), a.size()); + } + + void insertingAFolioChangesNothing() + { + QDomDocument moved = fixture(); + QDomElement first = diagrams(moved).first(); + QDomElement blank = first.cloneNode(false).toElement(); + blank.setAttribute(QStringLiteral("title"), QStringLiteral("new")); + blank.appendChild(moved.createElement(QStringLiteral("elements"))); + moved.documentElement().insertBefore(blank, first); + + const QMultiHash after = symbolUuids(resave(moved)); + QCOMPARE(after.size(), 9); + QCOMPARE(after, symbolUuids(resave(fixture()))); + } + + void savedUuidsAreKept() + { + QDomDocument doc = load(QFINDTESTDATA("fixtures/qet_bug_repro_resaved.qet")); + QCOMPARE(symbolUuids(resave(doc)), symbolUuids(doc)); + } + + void stackedIdenticalSymbolsDiffer() + { + QDomDocument doc = fixture(); + QDomElement one = symbols(diagrams(doc).first()).first(); + QDomElement copy = one.cloneNode(true).toElement(); + //A copy's terminals carry their own file ids; a symbol whose + //terminal ids are already taken is not loaded at all. + const QDomNodeList terminals = copy.elementsByTagName(QStringLiteral("terminal")); + for (int i = 0; i < terminals.size(); ++i) + terminals.at(i).toElement().setAttribute(QStringLiteral("id"), 90000 + i); + one.parentNode().appendChild(copy); + + const QStringList both = symbolUuids(resave(doc)).values( + QStringList{one.attribute(QStringLiteral("type")), + one.attribute(QStringLiteral("x")), + one.attribute(QStringLiteral("y")), + one.attribute(QStringLiteral("orientation"))} + .join(QLatin1Char('|'))); + QCOMPARE(both.size(), 2); + QVERIFY(both.at(0) != both.at(1)); + } +}; + +QTEST_APPLESS_MAIN(tst_derivedsymboluuid) + +#include "tst_derivedsymboluuid.moc" From e12410e439b42c914a0ce14fdb1a87596dbf72c1 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Mon, 28 Sep 2026 20:46:01 +1300 Subject: [PATCH 2/7] Never derive a uuid the file already carries A symbol keeps its derived uuid once saved, even when it is moved. A symbol saved without a uuid that later turns up on the spot it left -- a hand edit, an older version, another tool writing the file -- derived the same uuid, and the project had two symbols with one identity. readDiagramsXml() now collects every symbol and wire uuid the file carries, on any folio, before a folio loads; derivedItemUuid() moves to the next counter value while a candidate is among them. The result still depends on the file alone. Renamed from derivedUuid(), which QETProject already has for the project's own uuid. tst_derivedsymboluuid: newcomerOnAMovedSymbolsSpotGetsAnotherUuid fails with the check switched off. Co-Authored-By: Claude Opus 5.5 --- sources/diagram.cpp | 2 +- sources/qetproject.cpp | 37 ++++++++++++++++++++++---- sources/qetproject.h | 4 ++- tests/qttest/tst_derivedsymboluuid.cpp | 22 +++++++++++++++ 4 files changed, 58 insertions(+), 7 deletions(-) diff --git a/sources/diagram.cpp b/sources/diagram.cpp index bc0cd7c9e..ee3b6540b 100644 --- a/sources/diagram.cpp +++ b/sources/diagram.cpp @@ -1685,7 +1685,7 @@ bool Diagram::fromXml(QDomElement &document, //uuids anyway. if (consider_informations && m_project && QUuid(element_xml.attribute(QStringLiteral("uuid"))).isNull()) { - nvel_elmt->setUuid(m_project->derivedUuid( + nvel_elmt->setUuid(m_project->derivedItemUuid( QStringLiteral("element"), QStringList{type_id, element_xml.attribute(QStringLiteral("x")), diff --git a/sources/qetproject.cpp b/sources/qetproject.cpp index e66714577..b89acc626 100644 --- a/sources/qetproject.cpp +++ b/sources/qetproject.cpp @@ -276,7 +276,7 @@ QUuid QETProject::uuid() const } /** - @brief QETProject::derivedUuid + @brief QETProject::derivedItemUuid A uuid for an item of this project that was saved without one, the same on every load of the same file. @p key describes the item by what it is, never by its place in the file @@ -284,16 +284,31 @@ QUuid QETProject::uuid() const Items with the same @p kind and @p key anywhere in the project (a copied folio, two identical symbols stacked on one spot) are told apart by a counter, in load order among those items alone. + + A derived uuid is never one the file already carries: an item saved with + a derived uuid and then moved or re-connected keeps it, so a newcomer + later taking its old place or ends would otherwise derive the same one. + The file's saved uuids are collected before any folio loads + (readDiagramsXml()), so the result still depends on the file alone. + + uuids are unique within one project; copies of a project share them, as + they share every saved uuid. Anything bringing items in from another + project must renew them, as paste does. @return a UUID v5, which cannot collide with the v4 uuids given to new items */ -QUuid QETProject::derivedUuid(const QString &kind, const QString &key) +QUuid QETProject::derivedItemUuid(const QString &kind, const QString &key) { static const QUuid derived_ns(QStringLiteral("{7d1e9c3a-5b2f-4e8a-9c61-2f4b8d0e6a17}")); const QString full = kind + QLatin1Char('\n') + key; - const int n = m_derived_uuid_keys[full]++; - return QUuid::createUuidV5(derived_ns, - n ? full + QLatin1Char('\n') + QString::number(n) : full); + int &n = m_derived_uuid_keys[full]; + QUuid uuid; + do { + uuid = QUuid::createUuidV5(derived_ns, + n ? full + QLatin1Char('\n') + QString::number(n) : full); + ++n; + } while (m_saved_item_uuids.contains(uuid)); + return uuid; } /** @@ -1909,6 +1924,18 @@ void QETProject::readDiagramsXml(QDomDocument &xml_project) //Search the diagrams in the project QDomNodeList diagram_nodes = xml_project.elementsByTagName(QStringLiteral("diagram")); + //Every symbol and wire uuid the file already carries, on any folio, + //before a folio derives one for an item saved without: see + //derivedItemUuid(). + for (const QString &tag : {QStringLiteral("element"), QStringLiteral("conductor")}) { + const QDomNodeList nodes = xml_project.elementsByTagName(tag); + for (int i = 0; i < nodes.size(); ++i) { + const QUuid saved(nodes.at(i).toElement().attribute(QStringLiteral("uuid"))); + if (!saved.isNull()) + m_saved_item_uuids.insert(saved); + } + } + if(dlgWaiting) dlgWaiting->setProgressBarRange(0, diagram_nodes.length()*3); diff --git a/sources/qetproject.h b/sources/qetproject.h index 991b9f1a9..35830912c 100644 --- a/sources/qetproject.h +++ b/sources/qetproject.h @@ -36,6 +36,7 @@ #endif #include +#include #include class Diagram; @@ -107,7 +108,7 @@ class QETProject : public QObject ProjectPropertiesHandler& projectPropertiesHandler(); projectDataBase *dataBase(); QUuid uuid() const; - QUuid derivedUuid(const QString &kind, const QString &key); + QUuid derivedItemUuid(const QString &kind, const QString &key); ProjectState state() const; QList diagrams() const; int folioIndex(const Diagram *) const; @@ -367,6 +368,7 @@ class QETProject : public QObject KAutoSaveFile m_backup_file; QUuid m_uuid = QUuid::createUuid(); QHash m_derived_uuid_keys; + QSet m_saved_item_uuids; //symbol and wire uuids the file carries, see derivedItemUuid() projectDataBase m_data_base; QVector m_terminal_strip_vector; diff --git a/tests/qttest/tst_derivedsymboluuid.cpp b/tests/qttest/tst_derivedsymboluuid.cpp index d3fa5861a..e5a0e67f8 100644 --- a/tests/qttest/tst_derivedsymboluuid.cpp +++ b/tests/qttest/tst_derivedsymboluuid.cpp @@ -156,6 +156,28 @@ private slots: QCOMPARE(symbolUuids(resave(doc)), symbolUuids(doc)); } + // A symbol keeps its derived uuid once saved, even when moved. A symbol + // saved without a uuid that later turns up on the spot it left (a hand + // edit, an older version, another tool) would derive the same uuid: it + // must get another one instead. + void newcomerOnAMovedSymbolsSpotGetsAnotherUuid() + { + QDomDocument doc = resave(fixture()); + QDomElement moved = symbols(diagrams(doc).first()).first(); + QVERIFY(!QUuid(moved.attribute(QStringLiteral("uuid"))).isNull()); + QDomElement newcomer = moved.cloneNode(true).toElement(); + newcomer.removeAttribute(QStringLiteral("uuid")); + const QDomNodeList terminals = newcomer.elementsByTagName(QStringLiteral("terminal")); + for (int i = 0; i < terminals.size(); ++i) + terminals.at(i).toElement().setAttribute(QStringLiteral("id"), 90000 + i); + moved.setAttribute(QStringLiteral("x"), moved.attribute(QStringLiteral("x")).toInt() + 500); + moved.parentNode().appendChild(newcomer); + + const QMultiHash after = symbolUuids(resave(doc)); + QCOMPARE(after.size(), 10); + QCOMPARE(QSet(after.begin(), after.end()).size(), 10); + } + void stackedIdenticalSymbolsDiffer() { QDomDocument doc = fixture(); From 75450d7102f8d45696327e290391f4e92ffcbba5 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Mon, 28 Sep 2026 21:59:02 +1300 Subject: [PATCH 3/7] Make saving a just-saved project change nothing Saving a project that had just been saved changed it again in 18 of the 24 example projects, so a project kept in version control showed changes nobody made. Both causes were cleanup done on save but not on load: - Symbol information whose values were all empty was written as an empty block (DiagramContext::toXml() skips empty values, Element::toXml() wrote the block anyway). The next load read it as no information and the next save dropped it. The block is now written only when something went into it. - Information values were trimmed on save but not on load, so a label with stray spaces (" PRISE") kept them in memory and in its displayed copy until the project was opened again. The same rule, kept in one place, now applies when reading: stray whitespace around real content trimmed, a value that is only whitespace kept (#973). All 24 examples now save identically a second time (master: 6), and each one's first save is byte-for-byte what master wrote only on its second. A title-block property set to a single space keeps it through two saves. tst_resaveunchanged runs --resave twice on Projet_vierge.qet and m_000.qet; both fail without this change. Co-Authored-By: Claude Opus 5.5 --- sources/diagramcontext.cpp | 36 +++++++++---- sources/qetgraphicsitem/element.cpp | 6 ++- tests/qttest/CMakeLists.txt | 13 +++++ tests/qttest/tst_resaveunchanged.cpp | 81 ++++++++++++++++++++++++++++ 4 files changed, 124 insertions(+), 12 deletions(-) create mode 100644 tests/qttest/tst_resaveunchanged.cpp diff --git a/sources/diagramcontext.cpp b/sources/diagramcontext.cpp index 5137a7bf5..94b8696a2 100644 --- a/sources/diagramcontext.cpp +++ b/sources/diagramcontext.cpp @@ -144,6 +144,27 @@ bool DiagramContext::operator!=(const DiagramContext &dc) const return(!(*this == dc)); } +namespace { +/** + The value as it is saved, and so as it is read back: stray leading and + trailing whitespace around real content trimmed, but not a value that IS + whitespace -- unconditionally trimming an all-whitespace string collapses + it to "", which is silently indistinguishable from a value that was never + set. A title-block custom variable set to a single space -- a workaround + for #973, where an unset variable renders as its own literal placeholder + -- would otherwise vanish on the very next save. + Applied when reading as well as when writing, so that what is in memory + after a load is what the next save writes: a label shown from an + untrimmed value would otherwise keep its spaces on screen and in its + displayed copy until the project was saved and opened again, and a + just-saved project would change on its second save. +*/ +QString storedValue(const QString &raw) +{ + return raw.trimmed().isEmpty() ? raw : raw.trimmed(); +} +} // namespace + /** Export this context properties under the \a e XML element, using tags named \a tag_name (defaults to "property"). @@ -161,15 +182,7 @@ void DiagramContext::toXml(QDomElement &e, const QString &tag_name) const property.removeAttribute("name"); property.setAttribute("show", m_content_show[key]); property.setAttribute("name", key); - // Trim stray leading/trailing whitespace around real content, but - // not a value that IS whitespace: unconditionally trimming an - // all-whitespace string collapses it to "", which is silently - // indistinguishable from a value that was never set. A title-block - // custom variable set to a single space -- a workaround for #973, - // where an unset variable renders as its own literal placeholder -- - // would otherwise vanish on the very next save. - const QString stored = raw.trimmed().isEmpty() ? raw : raw.trimmed(); - QDomText value = e.ownerDocument().createTextNode(stored); + QDomText value = e.ownerDocument().createTextNode(storedValue(raw)); property.appendChild(value); e.appendChild(property); } @@ -182,7 +195,7 @@ void DiagramContext::toXml(QDomElement &e, const QString &tag_name) const void DiagramContext::fromXml(const QDomElement &e, const QString &tag_name) { foreach (QDomElement property, QET::findInDomElement(e, tag_name)) { if (!property.hasAttribute("name")) continue; - addValue(property.attribute("name"), QVariant(property.text())); + addValue(property.attribute("name"), QVariant(storedValue(property.text()))); m_content_show.insert(property.attribute("name"), property.attribute("show", "1").toInt()); } } @@ -198,7 +211,8 @@ void DiagramContext::fromXml(const pugi::xml_node &dom_element, const QString &t { for(auto node = dom_element.child(tag_name.toStdString().c_str()) ; node ; node = node.next_sibling(tag_name.toStdString().c_str())) { - addValue(node.attribute("name").as_string(), QVariant(node.text().as_string())); + addValue(node.attribute("name").as_string(), + QVariant(storedValue(QString::fromUtf8(node.text().as_string())))); m_content_show.insert(node.attribute("name").as_string(), node.attribute("show").empty()? 1 : node.attribute("show").as_int()); } } diff --git a/sources/qetgraphicsitem/element.cpp b/sources/qetgraphicsitem/element.cpp index fbbc3df5d..eb9dbfbe2 100644 --- a/sources/qetgraphicsitem/element.cpp +++ b/sources/qetgraphicsitem/element.cpp @@ -1033,7 +1033,11 @@ QDomElement Element::toXml( QDomElement infos = document.createElement(QStringLiteral("elementInformations")); m_data.m_informations.toXml(infos, QStringLiteral("elementInformation")); - element.appendChild(infos); + //toXml() skips empty values: an element whose information is + //all empty would otherwise be written an empty block, which the + //next load reads as no information and the next save drops. + if (infos.hasChildNodes()) + element.appendChild(infos); } //Save override properties (For now, only used when the element is a terminal) diff --git a/tests/qttest/CMakeLists.txt b/tests/qttest/CMakeLists.txt index 94283cc09..525ccb8cd 100644 --- a/tests/qttest/CMakeLists.txt +++ b/tests/qttest/CMakeLists.txt @@ -328,3 +328,16 @@ target_include_directories(tst_conductorselfretrace PRIVATE ${QET_DIR}/sources) target_link_libraries(tst_conductorselfretrace PRIVATE Qt::Test) target_compile_definitions(tst_conductorselfretrace PRIVATE "QET_TEST_BINARY_PATH=\"$\"") + +# Saving a project that was just saved changes nothing: runs the real +# binary's --resave twice on examples/Projet_vierge.qet (empty information +# values) and examples/m_000.qet (information values with stray spaces). +add_executable( + tst_resaveunchanged + tst_resaveunchanged.cpp) +add_test(NAME tst_resaveunchanged COMMAND tst_resaveunchanged) +add_dependencies(tst_resaveunchanged qelectrotech) +target_link_libraries(tst_resaveunchanged PRIVATE Qt::Test) +target_compile_definitions(tst_resaveunchanged PRIVATE + "QET_TEST_BINARY_PATH=\"$\"" + "QET_EXAMPLES_DIR=\"${QET_DIR}/examples\"") diff --git a/tests/qttest/tst_resaveunchanged.cpp b/tests/qttest/tst_resaveunchanged.cpp new file mode 100644 index 000000000..aff0fe360 --- /dev/null +++ b/tests/qttest/tst_resaveunchanged.cpp @@ -0,0 +1,81 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +#include + +#include +#include +#include +#include +#include + +// Saving a project that was just saved must change nothing. Two things +// made the second save differ from the first, both cleanup done on save +// but not on load: +// - symbol information whose values were all empty was written as an +// empty block, which the next load read as no +// information and the next save dropped (Projet_vierge.qet); +// - information values were trimmed on save but not on load, so a label +// with stray spaces kept them in its displayed copy until the project +// was opened again (m_000.qet). +// Runs the real binary's --resave twice on each example. +class tst_resaveunchanged : public QObject +{ + Q_OBJECT + + QTemporaryDir m_dir; + int m_run = 0; + + // --resave @p in to a new file, in a sandbox of its own (so a running + // QElectroTech cannot answer instead); returns the new file's path. + QString resave(const QString &in) + { + const QString out = m_dir.filePath(QStringLiteral("out%1.qet").arg(m_run)); + 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("--resave"), in, out}); + if (!proc.waitForFinished(120000) || proc.exitCode() != 0) return {}; + return out; + } + + static QByteArray read(const QString &path) + { + QFile f(path); + return f.open(QIODevice::ReadOnly) ? f.readAll() : QByteArray(); + } + +private slots: + void initTestCase() + { + QVERIFY(m_dir.isValid()); + QVERIFY(QFile::exists(QStringLiteral(QET_TEST_BINARY_PATH))); + } + + void secondSaveChangesNothing_data() + { + QTest::addColumn("project"); + QTest::newRow("empty information values") << QStringLiteral("Projet_vierge.qet"); + QTest::newRow("information values with stray spaces") << QStringLiteral("m_000.qet"); + } + + void secondSaveChangesNothing() + { + QFETCH(QString, project); + const QString first = resave(QStringLiteral(QET_EXAMPLES_DIR "/") + project); + QVERIFY2(!first.isEmpty(), "first --resave failed"); + const QString second = resave(first); + QVERIFY2(!second.isEmpty(), "second --resave failed"); + const QByteArray a = read(first), b = read(second); + QVERIFY(!a.isEmpty()); + QVERIFY2(a == b, "the second save changed the file"); + } +}; + +QTEST_APPLESS_MAIN(tst_resaveunchanged) + +#include "tst_resaveunchanged.moc" From 6ed41bbfd0bcd13e0c21589c5b85a02f72c10f0b Mon Sep 17 00:00:00 2001 From: ispyisail Date: Mon, 28 Sep 2026 22:33:28 +1300 Subject: [PATCH 4/7] Click again on a member of a selected group to pick it on its own Clicking an item of a group selects the whole group (#1070). Clicking again on one of its items is meant to select just that item, to edit it on its own -- what discussion #1070 proposed -- but the second click selected the whole group again: Qt left only the clicked item selected on release, and the group completion pulled the others back in. A press on a member of a group that is selected whole now notes that member (ItemGroups::memberToPick(), which also finds the member when the click lands on a symbol's own text). If the click ends without a drag and Qt has left only that member selected, the selection stays so. A drag still moves the whole group; Ctrl+click keeps its meaning; a group of one is not picked from. In the GUI, on two grouped texts: one click then Delete removes both (master and this); click, click again, Delete removes only the clicked text here, both on master; dragging after one click moves both texts by the same amount on both. tst_itemgroups: 5 new checks; removing the whole-group or the group-of-one condition fails one each. ctest 24/24. Co-Authored-By: Claude Opus 5.5 --- sources/diagram.cpp | 31 ++++++++++++++++++++++++ sources/diagram.h | 3 +++ sources/itemgroups.cpp | 41 ++++++++++++++++++++++++++++++++ sources/itemgroups.h | 3 +++ tests/qttest/tst_itemgroups.cpp | 42 +++++++++++++++++++++++++++++++++ 5 files changed, 120 insertions(+) diff --git a/sources/diagram.cpp b/sources/diagram.cpp index 4831b6484..498a82341 100644 --- a/sources/diagram.cpp +++ b/sources/diagram.cpp @@ -437,6 +437,24 @@ void Diagram::mousePressEvent(QGraphicsSceneMouseEvent *event) } rememberSelection(); + //Clicking again on a member of a group that is selected whole picks + //that member out, to edit it on its own (discussion #1070): noted + //here, decided on release, since a drag must still move the group. + //Ctrl keeps its usual meaning. + m_member_to_pick.clear(); + if (event->button() == Qt::LeftButton + && !event->modifiers().testFlag(Qt::ControlModifier)) { + QTransform view_transform; + if (event->widget()) { + if (auto view = qobject_cast(event->widget()->parentWidget())) { + view_transform = view->transform(); + } + } + if (QGraphicsItem *member = ItemGroups::memberToPick( + itemAt(event->scenePos(), view_transform))) { + m_member_to_pick = member->toGraphicsObject(); + } + } QGraphicsScene::mousePressEvent(event); completeGroupSelection(); } @@ -477,6 +495,19 @@ void Diagram::mouseReleaseEvent(QGraphicsSceneMouseEvent *event) } QGraphicsScene::mouseReleaseEvent(event); + + //A click that did not drag, on a member of a group selected whole: + //Qt has left only that member selected, and it stays so. + QGraphicsObject *picked = m_member_to_pick.data(); + m_member_to_pick.clear(); + if (picked + && (event->screenPos() - event->buttonDownScreenPos(Qt::LeftButton)).manhattanLength() + < QApplication::startDragDistance() + && selectedItems() == QList{picked}) { + rememberSelection(); + return; + } + //A click on an already selected item changes the selection on //release, not on press (Ctrl toggles it, a plain click keeps only it). completeGroupSelection(); diff --git a/sources/diagram.h b/sources/diagram.h index 8c2b453d6..83d408835 100644 --- a/sources/diagram.h +++ b/sources/diagram.h @@ -142,6 +142,9 @@ class Diagram : public QGraphicsScene //Selection before the current click, see completeGroupSelection() QList> m_previous_selection; void rememberSelection(); + //Member of a wholly selected group under the current click, which + //the click picks out on its own if it ends without a drag + QPointer m_member_to_pick; bool uuidUsedByOtherDiagram(const QUuid &uuid) const; QUuid derivedUuid(const QDomElement &root, const QString &reason) const; diff --git a/sources/itemgroups.cpp b/sources/itemgroups.cpp index 4e4c7ac9d..60da47593 100644 --- a/sources/itemgroups.cpp +++ b/sources/itemgroups.cpp @@ -62,6 +62,47 @@ QUuid ItemGroups::read(const QDomElement &xml) return QUuid(xml.attribute(QString::fromLatin1(xml_attribute))); } +/** + @return @a item, or its nearest ancestor, that belongs to a group -- a + click on a symbol's own text hits the text, but the symbol is the member + -- or nullptr if none does. +*/ +QGraphicsItem *ItemGroups::groupedItem(QGraphicsItem *item) +{ + for (; item; item = item->parentItem()) { + if (!groupOf(item).isNull()) { + return item; + } + } + return nullptr; +} + +/** + @return the member a click on @a hit may pick out on its own: the grouped + item hit, when every member of its group is selected already. A first + click selects the whole group; a second click, on a member of the group + it selected, is how the user asks for that member alone (discussion + #1070). nullptr when the click is not that. +*/ +QGraphicsItem *ItemGroups::memberToPick(QGraphicsItem *hit) +{ + QGraphicsItem *member = groupedItem(hit); + if (!member || !member->isSelected() || !member->scene()) { + return nullptr; + } + const QUuid group = groupOf(member); + int members = 0; + for (QGraphicsItem *item : member->scene()->items()) { + if (groupOf(item) == group) { + if (!item->isSelected()) { + return nullptr; + } + ++members; + } + } + return members > 1 ? member : nullptr; +} + /** Make the selection of @a scene whole groups again after it changed. A group with a selected member is selected entirely, except when the diff --git a/sources/itemgroups.h b/sources/itemgroups.h index 7e3c944b1..7e785bd8c 100644 --- a/sources/itemgroups.h +++ b/sources/itemgroups.h @@ -52,6 +52,9 @@ namespace ItemGroups void write(QDomElement &xml, const QGraphicsItem *item); QUuid read(const QDomElement &xml); + QGraphicsItem *groupedItem(QGraphicsItem *item); + QGraphicsItem *memberToPick(QGraphicsItem *hit); + bool completeSelection(QGraphicsScene *scene, const QList &previous, bool toggling); diff --git a/tests/qttest/tst_itemgroups.cpp b/tests/qttest/tst_itemgroups.cpp index 079c8c94c..486c8b82b 100644 --- a/tests/qttest/tst_itemgroups.cpp +++ b/tests/qttest/tst_itemgroups.cpp @@ -121,6 +121,48 @@ private slots: b = nullptr; } + // A second click on a member of a group selected whole picks that member + // out; a click on a member of a group not selected whole does not. + void aMemberOfAWholeGroupCanBePicked() + { + select({a, b}); + QCOMPARE(ItemGroups::memberToPick(a), a); + QCOMPARE(ItemGroups::memberToPick(b), b); + } + + void aMemberOfAPartlySelectedGroupIsNotPicked() + { + select({a}); // after one member was picked + QCOMPARE(ItemGroups::memberToPick(a), nullptr); + select({}); + QCOMPARE(ItemGroups::memberToPick(a), nullptr); + } + + void anUngroupedItemIsNotPicked() + { + select({c}); + QCOMPARE(ItemGroups::memberToPick(c), nullptr); + QCOMPARE(ItemGroups::memberToPick(nullptr), nullptr); + } + + void aGroupOfOneIsNotPicked() + { + ItemGroups::setGroup(e, QUuid()); // g2 is now d alone + select({d}); + QCOMPARE(ItemGroups::memberToPick(d), nullptr); + } + + // A click lands on a symbol's own text, not on the symbol: the member is + // the nearest grouped ancestor. + void aClickOnAMembersChildPicksTheMember() + { + auto child = new QGraphicsRectItem(0, 0, 2, 2, a); + QCOMPARE(ItemGroups::groupedItem(child), a); + select({a, b}); + QCOMPARE(ItemGroups::memberToPick(child), a); + QCOMPARE(ItemGroups::groupedItem(c), nullptr); + } + void xmlRoundTrip() { QDomDocument doc; From a8f940505da151c4506680945cc47b9d77ae6ebb Mon Sep 17 00:00:00 2001 From: ispyisail Date: Mon, 28 Sep 2026 22:39:03 +1300 Subject: [PATCH 5/7] Rotate a selected group as one piece Discussion #1070 proposed that rotate, like move, copy and delete, works on the whole group once one of its items is clicked. Rotate (Space) turned each member on its own spot instead, so rotating a group pulled it apart: two grouped texts side by side ended up each turned in place, no longer side by side. When the selection is exactly one whole group -- wires aside, which follow their symbols -- Rotate now turns it as one piece around its centre, as "Pivoter le groupe" (Shift+Space) already does (ItemGroups::soleWholeGroup()). Any other selection, including a single member picked out of its group, rotates as before. In the GUI, on two grouped texts selected by one click: Space on master leaves both where they were, turned; here it gives exactly what Shift+Space gives on both (both texts swung around the group's centre). tst_itemgroups: 4 new checks; without the whole-group condition, a picked member counts as a group and fails. ctest 24/24. Co-Authored-By: Claude Opus 5.5 --- sources/itemgroups.cpp | 31 +++++++++++++++++++++++++++++++ sources/itemgroups.h | 2 ++ sources/qetdiagrameditor.cpp | 13 ++++++++++++- tests/qttest/tst_itemgroups.cpp | 28 ++++++++++++++++++++++++++++ 4 files changed, 73 insertions(+), 1 deletion(-) diff --git a/sources/itemgroups.cpp b/sources/itemgroups.cpp index 4e4c7ac9d..ed742ec58 100644 --- a/sources/itemgroups.cpp +++ b/sources/itemgroups.cpp @@ -122,3 +122,34 @@ bool ItemGroups::completeSelection(QGraphicsScene *scene, } return changed; } + +/** + @return the group @a selected is exactly, whole -- every item in it + belongs to that group and every member of the group is in it -- or a null + uuid. Rotating such a selection turns the group as one piece rather than + each member in place (discussion #1070). A member picked out on its own + is not a whole group, and turns in place. + @param selected : the selected items that can be members (the caller + leaves out wires, which follow their symbols) +*/ +QUuid ItemGroups::soleWholeGroup(const QList &selected) +{ + if (selected.isEmpty() || !selected.first()->scene()) { + return QUuid(); + } + const QUuid group = groupOf(selected.first()); + if (group.isNull()) { + return QUuid(); + } + for (QGraphicsItem *item : selected) { + if (groupOf(item) != group) { + return QUuid(); + } + } + for (QGraphicsItem *item : selected.first()->scene()->items()) { + if (groupOf(item) == group && !item->isSelected()) { + return QUuid(); + } + } + return group; +} diff --git a/sources/itemgroups.h b/sources/itemgroups.h index 7e3c944b1..d4e18099d 100644 --- a/sources/itemgroups.h +++ b/sources/itemgroups.h @@ -55,6 +55,8 @@ namespace ItemGroups bool completeSelection(QGraphicsScene *scene, const QList &previous, bool toggling); + + QUuid soleWholeGroup(const QList &selected); } #endif // ITEMGROUPS_H diff --git a/sources/qetdiagrameditor.cpp b/sources/qetdiagrameditor.cpp index d65d94b95..80d52f4dd 100644 --- a/sources/qetdiagrameditor.cpp +++ b/sources/qetdiagrameditor.cpp @@ -25,6 +25,7 @@ #include "ElementsCollection/elementpickerpopup.h" #include "shortcutbarsettings.h" #include "qetgraphicsitem/conductor.h" +#include "itemgroups.h" #include "commandsearchpopup.h" #include "QWidgetAnimation/qwidgetanimation.h" #include "autoNum/ui/autonumberingdockwidget.h" @@ -2078,7 +2079,17 @@ void QETDiagramEditor::selectionGroupTriggered(QAction *action) } else if (value == "rotate_selection") { - RotateSelectionCommand *c = new RotateSelectionCommand(diagram); + //A selection that is exactly one whole group turns as one piece, + //as "Pivoter le groupe" does, rather than each member in place + //(discussion #1070). Wires follow their symbols either way. + QList members; + for (QGraphicsItem *item : diagram->selectedItems()) { + if (item->type() != Conductor::Type) { + members << item; + } + } + const bool whole_group = !ItemGroups::soleWholeGroup(members).isNull(); + RotateSelectionCommand *c = new RotateSelectionCommand(diagram, 90, nullptr, whole_group); if(c->isValid()) diagram->undoStack().push(c); } diff --git a/tests/qttest/tst_itemgroups.cpp b/tests/qttest/tst_itemgroups.cpp index 079c8c94c..b85cadc27 100644 --- a/tests/qttest/tst_itemgroups.cpp +++ b/tests/qttest/tst_itemgroups.cpp @@ -136,6 +136,34 @@ private slots: ItemGroups::setGroup(a, QUuid()); QVERIFY(ItemGroups::groupOf(a).isNull()); } + + // Rotate turns a selection that is exactly one whole group as one piece. + void aWholeGroupAloneIsASoleWholeGroup() + { + select({a, b}); + QCOMPARE(ItemGroups::soleWholeGroup(selection()), g1); + } + + void aPickedMemberIsNotAWholeGroup() + { + select({a}); + QVERIFY(ItemGroups::soleWholeGroup(selection()).isNull()); + } + + void aGroupWithOtherItemsIsNotASoleGroup() + { + select({a, b, c}); + QVERIFY(ItemGroups::soleWholeGroup(selection()).isNull()); + select({a, b, d, e}); + QVERIFY(ItemGroups::soleWholeGroup(selection()).isNull()); + } + + void ungroupedItemsAreNotAGroup() + { + select({c}); + QVERIFY(ItemGroups::soleWholeGroup(selection()).isNull()); + QVERIFY(ItemGroups::soleWholeGroup({}).isNull()); + } }; QTEST_MAIN(tst_itemgroups) From e25ae3cbfbcfc0dafc2e034cfd6503dd66459e66 Mon Sep 17 00:00:00 2001 From: ispyisail Date: Mon, 28 Sep 2026 23:29:51 +1300 Subject: [PATCH 6/7] Test the second save on every example, a single-space value and accents Review of #1109 (scorpio810): - tst_resaveunchanged now resaves every .qet in examples/ twice instead of two of them (24 rows, ~40 s). Dropping the load-time trim now also fails affuteuse_250h.qet, which the two-file version missed. - New case singleSpaceValueKept: a title-block property set to one space survives two saves (#973), and an accented property comes back unchanged. Fails if qetproject.cpp stops parsing with PreserveSpacingOnlyNodes. - New tst_diagramcontext: the QDom reader (projects) and the pugixml reader (element definitions in the collection) return the same value for plain, stray-spaced, accented and non-Latin text. Fails if the pugixml path decodes as Latin-1 or either reader stops trimming. The pugixml reader still reads a single-space value as "": pugixml drops whitespace-only text unless parse_ws_pcdata is set, as it did before this PR. That reader only sees element definitions, never a project, so #973's title-block values do not go through it. Co-Authored-By: Claude Opus 5.5 --- tests/qttest/CMakeLists.txt | 18 ++++++- tests/qttest/tst_diagramcontext.cpp | 72 ++++++++++++++++++++++++++++ tests/qttest/tst_resaveunchanged.cpp | 48 +++++++++++++++++-- 3 files changed, 132 insertions(+), 6 deletions(-) create mode 100644 tests/qttest/tst_diagramcontext.cpp diff --git a/tests/qttest/CMakeLists.txt b/tests/qttest/CMakeLists.txt index 525ccb8cd..a32d385f4 100644 --- a/tests/qttest/CMakeLists.txt +++ b/tests/qttest/CMakeLists.txt @@ -330,8 +330,8 @@ target_compile_definitions(tst_conductorselfretrace PRIVATE "QET_TEST_BINARY_PATH=\"$\"") # Saving a project that was just saved changes nothing: runs the real -# binary's --resave twice on examples/Projet_vierge.qet (empty information -# values) and examples/m_000.qet (information values with stray spaces). +# binary's --resave twice on every project in examples/, and on one whose +# title block holds a single-space value (#973). add_executable( tst_resaveunchanged tst_resaveunchanged.cpp) @@ -341,3 +341,17 @@ target_link_libraries(tst_resaveunchanged PRIVATE Qt::Test) target_compile_definitions(tst_resaveunchanged 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. +add_executable( + tst_diagramcontext + tst_diagramcontext.cpp + ${QET_DIR}/sources/diagramcontext.cpp + ${QET_DIR}/sources/qet.cpp + ${QET_DIR}/sources/qeticons.cpp + ${QET_DIR}/sources/shortcutmanager.cpp) +add_test(NAME tst_diagramcontext COMMAND tst_diagramcontext) +target_include_directories(tst_diagramcontext PRIVATE ${QET_DIR}/sources) +target_link_libraries(tst_diagramcontext PRIVATE Qt::Test Qt::Widgets Qt::Xml pugixml::pugixml) diff --git a/tests/qttest/tst_diagramcontext.cpp b/tests/qttest/tst_diagramcontext.cpp new file mode 100644 index 000000000..f0e004b0d --- /dev/null +++ b/tests/qttest/tst_diagramcontext.cpp @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +#include + +#include "diagramcontext.h" +#include "qetapp.h" + +QString QETApp::m_interface_language; + +/** + DiagramContext::fromXml() has two readers: QDom, for projects, and + pugixml, for element definitions in the collection. Both must give the + values the next save writes -- stray spaces around real content trimmed, + accented characters kept. +*/ +class tst_diagramcontext : public QObject +{ + Q_OBJECT + + static QByteArray xml(const QString &value) + { + return QStringLiteral( + "" + "%1" + "") + .arg(value) + .toUtf8(); + } + + static QString fromDom(const QByteArray &data) + { + QDomDocument doc; + if (!doc.setContent(data)) return QStringLiteral(""); + DiagramContext dc; + dc.fromXml(doc.documentElement(), QStringLiteral("elementInformation")); + return dc.value(QStringLiteral("v")).toString(); + } + + static QString fromPugi(const QByteArray &data) + { + pugi::xml_document doc; + if (!doc.load_buffer(data.constData(), size_t(data.size()))) + return QStringLiteral(""); + DiagramContext dc; + dc.fromXml(doc.document_element(), QStringLiteral("elementInformation")); + return dc.value(QStringLiteral("v")).toString(); + } + +private slots: + void bothReadersAgree_data() + { + QTest::addColumn("value"); + QTest::addColumn("expected"); + + QTest::newRow("plain") << "PRISE" << "PRISE"; + QTest::newRow("stray spaces") << " PRISE " << "PRISE"; + QTest::newRow("accents") << "Armoire façade été" << "Armoire façade été"; + QTest::newRow("accents and stray spaces") << " Moteur à cage " << "Moteur à cage"; + QTest::newRow("non-Latin") << "Двигатель 電機" << "Двигатель 電機"; + } + + void bothReadersAgree() + { + QFETCH(QString, value); + QFETCH(QString, expected); + QCOMPARE(fromDom(xml(value)), expected); + QCOMPARE(fromPugi(xml(value)), expected); + } +}; + +QTEST_APPLESS_MAIN(tst_diagramcontext) + +#include "tst_diagramcontext.moc" diff --git a/tests/qttest/tst_resaveunchanged.cpp b/tests/qttest/tst_resaveunchanged.cpp index aff0fe360..55fed0942 100644 --- a/tests/qttest/tst_resaveunchanged.cpp +++ b/tests/qttest/tst_resaveunchanged.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include // Saving a project that was just saved must change nothing. Two things @@ -16,7 +17,8 @@ // - information values were trimmed on save but not on load, so a label // with stray spaces kept them in its displayed copy until the project // was opened again (m_000.qet). -// Runs the real binary's --resave twice on each example. +// Runs the real binary's --resave twice on every example, and on a +// project whose title block holds a value that is a single space (#973). class tst_resaveunchanged : public QObject { Q_OBJECT @@ -56,17 +58,23 @@ private slots: QVERIFY(QFile::exists(QStringLiteral(QET_TEST_BINARY_PATH))); } + // Projet_vierge.qet has empty information values, m_000.qet values + // with stray spaces; every other example is here so a new cause shows. void secondSaveChangesNothing_data() { QTest::addColumn("project"); - QTest::newRow("empty information values") << QStringLiteral("Projet_vierge.qet"); - QTest::newRow("information values with stray spaces") << QStringLiteral("m_000.qet"); + const QDir examples(QStringLiteral(QET_EXAMPLES_DIR)); + const QStringList projects = + examples.entryList({QStringLiteral("*.qet")}, QDir::Files, QDir::Name); + QVERIFY(!projects.isEmpty()); + for (const QString &project : projects) + QTest::newRow(project.toUtf8().constData()) << examples.filePath(project); } void secondSaveChangesNothing() { QFETCH(QString, project); - const QString first = resave(QStringLiteral(QET_EXAMPLES_DIR "/") + project); + const QString first = resave(project); QVERIFY2(!first.isEmpty(), "first --resave failed"); const QString second = resave(first); QVERIFY2(!second.isEmpty(), "second --resave failed"); @@ -74,6 +82,38 @@ private slots: QVERIFY(!a.isEmpty()); QVERIFY2(a == b, "the second save changed the file"); } + + // A title-block value that is a single space is kept through two saves + // (#973), and a value with accents comes back as it went in. + void singleSpaceValueKept() + { + QByteArray xml = read(QStringLiteral(QET_EXAMPLES_DIR "/Projet_vierge.qet")); + QVERIFY(xml.contains("")); + xml.replace("", + "" + " " + "Armoire façade été"); + const QString in = m_dir.filePath(QStringLiteral("space.qet")); + QFile f(in); + QVERIFY(f.open(QIODevice::WriteOnly)); + f.write(xml); + f.close(); + + const QString first = resave(in); + QVERIFY2(!first.isEmpty(), "first --resave failed"); + const QString second = resave(first); + QVERIFY2(!second.isEmpty(), "second --resave failed"); + const QByteArray a = read(first), b = read(second); + QVERIFY2(a == b, "the second save changed the file"); + + const QString saved = QString::fromUtf8(b); + QVERIFY2(saved.contains(QRegularExpression( + QStringLiteral("]*name=\"space\"[^>]*> "))), + "the single-space value was lost"); + QVERIFY2(saved.contains(QRegularExpression( + QStringLiteral("]*name=\"accents\"[^>]*>Armoire façade été"))), + "the accented value changed"); + } }; QTEST_APPLESS_MAIN(tst_resaveunchanged) From cdcff93190422388c58bfab284378721f19ac8dc Mon Sep 17 00:00:00 2001 From: Laurent Trinques Date: Mon, 28 Sep 2026 16:42:05 +0200 Subject: [PATCH 7/7] Update CMakeLists.txt --- tests/qttest/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/qttest/CMakeLists.txt b/tests/qttest/CMakeLists.txt index b1613d522..ca296b6f8 100644 --- a/tests/qttest/CMakeLists.txt +++ b/tests/qttest/CMakeLists.txt @@ -343,6 +343,7 @@ target_compile_definitions(tst_derivedsymboluuid PRIVATE # Saving a project that was just saved changes nothing: runs the real # binary's --resave twice on every project in examples/, and on one whose # title block holds a single-space value (#973). + add_executable( tst_resaveunchanged tst_resaveunchanged.cpp)