diff --git a/sources/diagram.cpp b/sources/diagram.cpp index 4831b6484..c0e89fdd2 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(); @@ -1676,6 +1707,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->derivedItemUuid( + 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/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/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/itemgroups.cpp b/sources/itemgroups.cpp index 4e4c7ac9d..8ad4c2256 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 @@ -122,3 +163,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..90840ffbd 100644 --- a/sources/itemgroups.h +++ b/sources/itemgroups.h @@ -52,9 +52,14 @@ 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); + + 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/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/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..b89acc626 100644 --- a/sources/qetproject.cpp +++ b/sources/qetproject.cpp @@ -275,6 +275,42 @@ QUuid QETProject::uuid() const return m_uuid; } +/** + @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 + 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. + + 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::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; + 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; +} + /** @brief QETProject::init */ @@ -1888,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 e05f3b216..35830912c 100644 --- a/sources/qetproject.h +++ b/sources/qetproject.h @@ -36,6 +36,7 @@ #endif #include +#include #include class Diagram; @@ -107,6 +108,7 @@ class QETProject : public QObject ProjectPropertiesHandler& projectPropertiesHandler(); projectDataBase *dataBase(); QUuid uuid() const; + QUuid derivedItemUuid(const QString &kind, const QString &key); ProjectState state() const; QList diagrams() const; int folioIndex(const Diagram *) const; @@ -365,6 +367,8 @@ class QETProject : public QObject QFuture m_backup_future; 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/CMakeLists.txt b/tests/qttest/CMakeLists.txt index 9456b5bf4..6bafd77fa 100644 --- a/tests/qttest/CMakeLists.txt +++ b/tests/qttest/CMakeLists.txt @@ -344,3 +344,42 @@ if(QET_HAS_SCRIPTING) target_compile_definitions(tst_scriptconductoruuid PRIVATE "QET_TEST_BINARY_PATH=\"$\"") endif() + +# 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=\"$\"") +# 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) +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\"") + +# 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_derivedsymboluuid.cpp b/tests/qttest/tst_derivedsymboluuid.cpp new file mode 100644 index 000000000..e5a0e67f8 --- /dev/null +++ b/tests/qttest/tst_derivedsymboluuid.cpp @@ -0,0 +1,206 @@ +// 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)); + } + + // 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(); + 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" 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_itemgroups.cpp b/tests/qttest/tst_itemgroups.cpp index 079c8c94c..217ea7d41 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; @@ -136,6 +178,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) diff --git a/tests/qttest/tst_resaveunchanged.cpp b/tests/qttest/tst_resaveunchanged.cpp new file mode 100644 index 000000000..55fed0942 --- /dev/null +++ b/tests/qttest/tst_resaveunchanged.cpp @@ -0,0 +1,121 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +#include + +#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 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 + + 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))); + } + + // 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"); + 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(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"); + } + + // 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) + +#include "tst_resaveunchanged.moc"