diff --git a/misc/qet-mcp/README.md b/misc/qet-mcp/README.md index f86f06860..c8e994d9b 100644 --- a/misc/qet-mcp/README.md +++ b/misc/qet-mcp/README.md @@ -296,6 +296,18 @@ Python, plus the hang guard on `addConductor` and the database refresh in an index would shift. A folio saved without a uuid shows it empty: QElectroTech gives it one on load and writes it on the next save, so it appears after a first `qet_edit`. Needs `qet.folioIndex()` in the build. +- **A conductor can be named by its uuid** (`qet_conductors` reports it): + `set_conductor`, `move_conductor_segment` and `delete_conductor` take + `"conductor": "{uuid}"` in place of `element` + `terminal`, which works + where two conductors meet at a terminal. It is turned at run time into an + end whose terminal carries only that conductor; where both of its ends + are shared the op fails and its `note` says why. Needs + `qet.conductorEnds()` in the build. A uuid names one wire, but + `set_conductor` still changes the whole potential, the same as by + terminal. A project saved before conductors carried a uuid has none in + the file: QElectroTech makes a new one on every load and does not save + it, so `qet_conductors` reports `uuid` as empty and those conductors are + named by `element` + `terminal` until wire uuids last (#1103). - **`qet_export` isolates its launch.** SingleApplication keys its socket on `applicationFilePath()`, so a second launch of the same binary path forwards its request to an already-running instance and returns *that* diff --git a/misc/qet-mcp/qet_mcp.py b/misc/qet-mcp/qet_mcp.py index dbf8bb79b..303a806d3 100755 --- a/misc/qet-mcp/qet_mcp.py +++ b/misc/qet-mcp/qet_mcp.py @@ -1355,6 +1355,10 @@ SEARCH_REPLACE_KINDS = ["element_info", "conductor", "text"] FOLIO_PROPERTIES = ["title", "author", "filename", "plant", "locmach", "indexrev", "folio", "template"] +# The ops that address one conductor by element + terminal, and so also +# take "conductor": "{uuid}" (qet_conductors reports each one's uuid). +CONDUCTOR_UUID_OPS = ("set_conductor", "move_conductor_segment", "delete_conductor") + # Accepted by set_conductor. The names are the project file's own, so what # a script sets is what qet_conductors reports back. CONDUCTOR_PROPERTIES = ["num", "formula", "function", "bus", "cable", @@ -1406,6 +1410,35 @@ def _build_script(operations: list, output: str) -> str: f"qet.log({_js(_MARKER)} + JSON.stringify(" "{kind: 'capabilities', missing: missing}));", "var stop = false;", + # A conductor named by uuid is turned into one of its ends, as the + # conductor calls take it: an end whose terminal carries no other + # conductor, so the call cannot pick the wrong one. null, with the + # reason logged, if it is not on the folio or both ends are shared. + "function qetMcpConductorEnd(index, folio, uuid) {", + " var ends = qet.conductorEnds(folio, uuid);", + " var why = 'no conductor ' + uuid + ' on folio ' + folio;", + " if (ends && ends.length === 2) {", + " var lines = qet.conductors(folio);", + " for (var k = 0; k < 2; k++) {", + " if (ends[k] === '?') continue;", + " var n = 0;", + " for (var j = 0; j < lines.length; j++) {", + " var p = lines[j].split(' : ')[0].split(' -- ');", + " if (p[0] === ends[k] || p[1] === ends[k]) n++;", + " }", + " if (n === 1) {", + " var m = ends[k].split(' terminal ');", + " return {element: m[0], terminal: parseInt(m[1], 10)};", + " }", + " }", + " why = 'conductor ' + uuid + ' shares both of its terminals with other '", + " + 'conductors; the conductor calls address one by a terminal carrying '", + " + 'only it';", + " }", + f" qet.log({_js(_MARKER)} + JSON.stringify(" + "{kind: 'op_note', index: index, note: why}));", + " return null;", + "}", "if (missing.length === 0) {", ] @@ -1572,6 +1605,25 @@ def _build_script(operations: list, output: str) -> str: if name == "add_shape" and op.get("shape") not in SHAPES: raise ValueError(f"operation {i}: unknown shape {op.get('shape')!r}; " f"expected one of {', '.join(SHAPES)}") + # The conductor ops take "conductor": "{uuid}" in place of element + + # terminal: a uuid names one conductor for good, where a terminal + # can carry several. + conductor_js = None + if name in CONDUCTOR_UUID_OPS and "conductor" in op: + if "element" in op or "terminal" in op: + raise ValueError(f"operation {i} ({name}): give either \"conductor\" " + f"(its uuid) or \"element\" + \"terminal\", not both") + if op["conductor"] == "": + raise ValueError(f"operation {i}: \"conductor\" is empty -- a conductor " + f"from a project saved before conductors carried a uuid " + f"has none in the file; name it by \"element\" + " + f"\"terminal\" instead") + if not isinstance(op["conductor"], str) or not _UUID_RE.fullmatch(op["conductor"]): + raise ValueError(f"operation {i}: \"conductor\" must be a conductor uuid, " + f"got {op['conductor']!r}") + conductor_js = _js(op["conductor"]) + uuid_methods.add("conductorEnds") + op = {**op, "element": "{00000000-0000-0000-0000-000000000000}", "terminal": 0} args = [] for key, kind in spec: if key not in op: @@ -1579,6 +1631,8 @@ def _build_script(operations: list, output: str) -> str: folio_js = args[0] if args else "0" element_js = args[1] if len(args) > 1 else None args.append(ref_or(op[key], kind, i, key)) + if conductor_js is not None: + args[1], args[2] = f"e{i}.element", f"e{i}.terminal" ident = op.get("id") if ident is not None: @@ -1590,6 +1644,9 @@ def _build_script(operations: list, output: str) -> str: call = "qet.addFolio()" if method is None else f"qet.{method}({', '.join(args)})" lines.append(" if (!stop) {") + if conductor_js is not None: + lines.append(f" var e{i} = qetMcpConductorEnd({i}, {args[0]}, {conductor_js});") + call = f"(e{i} ? {call} : false)" lines.append(f" var v{i} = {call};") if ident is not None: lines.append(f" R[{_js(ident)}] = v{i};") @@ -1635,7 +1692,7 @@ def _parse_script_output(text: str) -> dict: the cost is nothing and the failure it prevents is silent (an edit that worked, reported as having run no operations at all, which is what the first version of this tool did).""" - caps, ops, saved, stopped = None, [], None, False + caps, ops, saved, stopped, notes = None, [], None, False, {} for line in text.splitlines(): idx = line.find(_MARKER) if idx < 0: @@ -1654,9 +1711,14 @@ def _parse_script_output(text: str) -> dict: rec["succeeded"] = not (r is None or r is False or r == "" or r == [] or r == {} or (isinstance(r, int) and not isinstance(r, bool) and r == -1)) ops.append(rec) + elif rec.get("kind") == "op_note": + notes[rec.get("index")] = rec.get("note") elif rec.get("kind") == "save": saved = bool(rec.get("result")) stopped = bool(rec.get("stopped_early")) + for rec in ops: + if rec.get("index") in notes: + rec["note"] = notes[rec["index"]] return {"missing_methods": caps, "operations": ops, "saved": saved, "stopped_early": stopped} @@ -2507,8 +2569,15 @@ TOOLS = [ "index in the element definition; qet_element_info lists them. " "set_conductor addresses a conductor as the one on a given " "terminal and applies the change to its whole electrical " - "potential, so name a terminal carrying exactly one conductor; " - "its \"property\" is one of " + ", ".join(CONDUCTOR_PROPERTIES) + + "potential, so name a terminal carrying exactly one conductor, " + "or give \"conductor\": its uuid from qet_conductors in place " + "of \"element\" + \"terminal\" (set_conductor, " + "move_conductor_segment and delete_conductor all accept it; it " + "needs one of the conductor's two terminals to carry only it, " + "and a conductor qet_conductors lists with an empty uuid has " + "none to give). A uuid names one wire, but set_conductor still " + "changes its whole potential, as it does by terminal. " + "set_conductor's \"property\" is one of " + ", ".join(CONDUCTOR_PROPERTIES) + ". move_conductor_segment reroutes the drawn path itself rather " "than a property of the potential -- addressed the same way (a " "terminal carrying exactly one conductor), plus a segment index " diff --git a/misc/qet-mcp/test_qet_mcp.py b/misc/qet-mcp/test_qet_mcp.py index 8238a2ce4..a32f7bbee 100644 --- a/misc/qet-mcp/test_qet_mcp.py +++ b/misc/qet-mcp/test_qet_mcp.py @@ -213,6 +213,34 @@ class EditValidation(unittest.TestCase): self.build([{"op": "set_folio", "folio": "", "property": "author", "value": "a"}]) with self.assertRaisesRegex(ValueError, "folio index, its uuid"): self.build([{"op": "set_folio", "folio": "first", "property": "author", "value": "a"}]) + + def test_conductor_by_uuid(self): + """A conductor named by uuid is turned into one of its ends at run + time; the lookup is required only then, and "conductor" cannot be + given alongside element + terminal.""" + U = "{11111111-2222-4333-8444-555555555555}" + s = self.build([{"op": "delete_conductor", "folio": 1, "conductor": U}]) + self.assertIn(f'var e0 = qetMcpConductorEnd(0, 1, "{U}");', s) + self.assertIn("(e0 ? qet.deleteConductor(1, e0.element, e0.terminal) : false)", s) + self.assertIn('"conductorEnds"', s) + s = self.build([{"op": "set_conductor", "folio": 0, "conductor": U, + "property": "num", "value": "W1"}]) + self.assertIn('qet.setConductorProperty(0, e0.element, e0.terminal, "num", "W1")', s) + s = self.build([{"op": "move_conductor_segment", "folio": 2, "conductor": U, + "segment": 1, "dx": 10, "dy": 0}]) + self.assertIn(f'var e0 = qetMcpConductorEnd(0, 2, "{U}");', s) + self.assertIn("(e0 ? qet.moveConductorSegment(2, e0.element, e0.terminal, 1, 10, 0) : false)", s) + # an end whose terminal or element is missing is never picked + self.assertIn("if (ends[k] === '?') continue;", s) + self.assertNotIn('"conductorEnds"', self.build( + [{"op": "delete_conductor", "folio": 0, "element": U, "terminal": 0}])) + with self.assertRaisesRegex(ValueError, "not both"): + self.build([{"op": "delete_conductor", "folio": 0, "conductor": U, + "element": U, "terminal": 0}]) + with self.assertRaisesRegex(ValueError, "must be a conductor uuid"): + self.build([{"op": "delete_conductor", "folio": 0, "conductor": "W1"}]) + with self.assertRaisesRegex(ValueError, "saved before conductors carried a uuid"): + self.build([{"op": "delete_conductor", "folio": 0, "conductor": ""}]) def test_every_op_generates_a_script(self): # one minimal valid instance of every op @@ -2399,6 +2427,40 @@ class Integration(unittest.TestCase): r = self.ok(self.sb.edit(base, ops)) self.assertEqual([c["num"] for c in m.tool_conductors(r["output"])["conductors"]], ["W7"]) + def _hub(self): + """e0's terminal 0 carries two conductors, to e1 and to e2. Returns + the saved project and the two conductors' uuids.""" + base = self.sb.new() + r = self.ok(self.sb.edit(base, [ + {"op": "add_folio", "id": "f"}, + *[{"op": "add_element", "id": f"e{i}", "folio": "$f", "path": COIL, "x": 100 + i * 200, "y": 100} + for i in range(3)], + {"op": "add_conductor", "folio": "$f", "from": "$e0", "from_terminal": 0, "to": "$e1", "to_terminal": 0}, + {"op": "add_conductor", "folio": "$f", "from": "$e0", "from_terminal": 0, "to": "$e2", "to_terminal": 0}])) + uuids = [c["uuid"] for c in m.tool_conductors(r["output"])["conductors"]] + self.assertEqual(len(uuids), 2) + self.assertTrue(all(uuids), "new conductors carry a saved uuid") + return r["output"], uuids + + def test_conductor_by_uuid_where_two_meet_at_a_terminal(self): + """The terminal e0/0 carries both conductors, which element + terminal + cannot name; each uuid names one, whichever it is.""" + project, uuids = self._hub() + for gone, kept in ((uuids[0], uuids[1]), (uuids[1], uuids[0])): + with self.subTest(deleted=gone): + r = self.ok(self.sb.edit(project, [ + {"op": "delete_conductor", "folio": 1, "conductor": gone}], out="out2.qet")) + left = [c["uuid"] for c in m.tool_conductors(r["output"])["conductors"]] + self.assertEqual(left, [kept]) + + def test_unknown_conductor_uuid_is_reported(self): + project, _ = self._hub() + r = self.sb.edit(project, [{"op": "delete_conductor", "folio": 1, + "conductor": "{11111111-2222-4333-8444-555555555555}"}], + out="out2.qet") + self.assertFalse(r["ok"]) + self.assertIn("no conductor", r["operations"][0].get("note", "")) + # ---- cross references, strips, folios ---- def test_cross_reference_across_folios(self): diff --git a/sources/diagram.cpp b/sources/diagram.cpp index 4831b6484..008fa9495 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; } @@ -1808,6 +1856,28 @@ bool Diagram::fromXml(QDomElement &document, { addItem(c); c -> fromXml(f); + //A wire saved without a uuid got a random one that was + //never saved (#754), so it had no identity from one + //session to the next. Derive it from what it connects: + //the symbol and terminal at each end, sorted so the + //direction it was drawn in does not matter. Never its + //place in the file or its folio's index, so inserting or + //moving a folio, or saving the wires in another order, + //does not change it. It is saved from now on, so + //re-connecting the wire later keeps it; derivedItemUuid() + //never hands out a uuid the file already carries, so a + //wire later drawn on the ends it left gets another one. + if (consider_informations && m_project + && QUuid(f.attribute(QStringLiteral("uuid"))).isNull()) { + auto end = [](const Terminal *t) { + return t->parentElement()->uuid().toString() + + QLatin1Char('/') + t->stableUuid().toString(); + }; + QStringList ends{end(p1), end(p2)}; + ends.sort(); + c->setUuid(m_project->derivedItemUuid(QStringLiteral("conductor"), + ends.join(QLatin1Char('\n')))); + } added_conductors << c; } else 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/conductor.h b/sources/qetgraphicsitem/conductor.h index d1ecd6e43..c2f3232e5 100644 --- a/sources/qetgraphicsitem/conductor.h +++ b/sources/qetgraphicsitem/conductor.h @@ -80,6 +80,7 @@ class Conductor : public QGraphicsObject ConductorTextItem *textItem() const; QUuid uuid() const {return m_uuid;} void newUuid() {m_uuid = QUuid::createUuid(); m_persist_uuid = true;} //create new uuid for this conductor + void setUuid(const QUuid &uuid) {m_uuid = uuid; m_persist_uuid = true;} //saved from now on void updatePath(const QRectF & = QRectF()); //This method do nothing, it's only made to be used with Q_PROPERTY 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/sources/scripting/qetscriptapi.cpp b/sources/scripting/qetscriptapi.cpp index 3c7c4c549..8cbbd926a 100644 --- a/sources/scripting/qetscriptapi.cpp +++ b/sources/scripting/qetscriptapi.cpp @@ -867,6 +867,18 @@ bool QetScriptApi::addConductor(int folioIndex, return t1->isLinkedTo(t2); } +namespace { +/// "{element uuid} terminal N", the form conductors() prints an end in and +/// the conductor calls take as element uuid + terminal index. +QString describeEnd(Terminal *t) +{ + if (!t || !t->parentElement()) return QStringLiteral("?"); + return QStringLiteral("%1 terminal %2") + .arg(t->parentElement()->uuid().toString()) + .arg(t->parentElement()->terminals().indexOf(t)); +} +} // namespace + /** @brief QetScriptApi::conductors One line per conductor on the folio: which terminals it joins and its @@ -882,23 +894,57 @@ QStringList QetScriptApi::conductors(int folioIndex) const const QList diagrams = m_project->diagrams(); if (folioIndex < 0 || folioIndex >= diagrams.count()) return list; - auto describe = [](Terminal *t) -> QString { - if (!t || !t->parentElement()) return QStringLiteral("?"); - return QStringLiteral("%1 terminal %2") - .arg(t->parentElement()->uuid().toString()) - .arg(t->parentElement()->terminals().indexOf(t)); - }; - DiagramContent content(diagrams.at(folioIndex), false); const QList all = content.conductors(DiagramContent::AnyConductor); for (Conductor *c : all) { list << QStringLiteral("%1 -- %2 : num='%3'") - .arg(describe(c->terminal1), describe(c->terminal2), c->properties().text); + .arg(describeEnd(c->terminal1), describeEnd(c->terminal2), c->properties().text); } return list; } +/** + @brief QetScriptApi::conductorUuids + The uuid of every conductor on the folio, in the order conductors() + lists them. +*/ +QStringList QetScriptApi::conductorUuids(int folioIndex) const +{ + QStringList list; + if (!m_project) return list; + const QList diagrams = m_project->diagrams(); + if (folioIndex < 0 || folioIndex >= diagrams.count()) return list; + + DiagramContent content(diagrams.at(folioIndex), false); + for (Conductor *c : content.conductors(DiagramContent::AnyConductor)) + list << c->uuid().toString(); + return list; +} + +/** + @brief QetScriptApi::conductorEnds + The two ends of the conductor carrying @p uuid, each as + "{element uuid} terminal N" -- the element uuid and terminal index the + conductor calls take -- or an empty list if the folio has no such + conductor. A uuid names one conductor even where two meet at a terminal, + which an element uuid + terminal index cannot. +*/ +QStringList QetScriptApi::conductorEnds(int folioIndex, const QString &uuid) const +{ + if (!m_project) return {}; + const QList diagrams = m_project->diagrams(); + if (folioIndex < 0 || folioIndex >= diagrams.count()) return {}; + const QUuid wanted(uuid); + if (wanted.isNull()) return {}; + + DiagramContent content(diagrams.at(folioIndex), false); + for (Conductor *c : content.conductors(DiagramContent::AnyConductor)) + if (c->uuid() == wanted) + return {describeEnd(c->terminal1), describeEnd(c->terminal2)}; + return {}; +} + QString QetScriptApi::conductorProperty(int folioIndex, const QString &elementUuid, int terminalIndex, const QString &property) const { diff --git a/sources/scripting/qetscriptapi.h b/sources/scripting/qetscriptapi.h index 8aa634f01..62240935e 100644 --- a/sources/scripting/qetscriptapi.h +++ b/sources/scripting/qetscriptapi.h @@ -398,6 +398,8 @@ class QetScriptApi : public QObject // -- conductor properties, applied to the whole potential -- Q_INVOKABLE QStringList conductors(int folioIndex) const; + Q_INVOKABLE QStringList conductorUuids(int folioIndex) const; + Q_INVOKABLE QStringList conductorEnds(int folioIndex, const QString &uuid) const; Q_INVOKABLE QString conductorProperty(int folioIndex, const QString &elementUuid, int terminalIndex, const QString &property) const; Q_INVOKABLE bool setConductorProperty(int folioIndex, const QString &elementUuid, diff --git a/tests/qttest/CMakeLists.txt b/tests/qttest/CMakeLists.txt index 94283cc09..e2fde811c 100644 --- a/tests/qttest/CMakeLists.txt +++ b/tests/qttest/CMakeLists.txt @@ -328,3 +328,73 @@ 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=\"$\"") + +# qet.conductorUuids() / qet.conductorEnds(): a script lists a folio's +# conductors by uuid and finds each one's two ends. Runs a script through +# the real binary's --run on fixtures/qet_bug_repro_resaved.qet, so only +# where --run exists: a build without the Qt Qml module has no scripting +# (see QET_HAS_SCRIPTING in the top-level CMakeLists.txt). +if(QET_HAS_SCRIPTING) + add_executable( + tst_scriptconductoruuid + tst_scriptconductoruuid.cpp) + add_test(NAME tst_scriptconductoruuid COMMAND tst_scriptconductoruuid) + add_dependencies(tst_scriptconductoruuid qelectrotech) + target_link_libraries(tst_scriptconductoruuid PRIVATE Qt::Test) + 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=\"$\"") + +# A wire saved without a uuid gets one derived from its two ends: the same +# on every load, saved, and unchanged by inserting a folio, reordering the +# wires or drawing a wire the other way. Runs the real binary's --resave on +# a fixture and on examples/tremie_vibrante.qet (ends by terminal number). +add_executable( + tst_derivedwireuuid + tst_derivedwireuuid.cpp) +add_test(NAME tst_derivedwireuuid COMMAND tst_derivedwireuuid) +add_dependencies(tst_derivedwireuuid qelectrotech) +target_link_libraries(tst_derivedwireuuid PRIVATE Qt::Test Qt::Xml) +target_compile_definitions(tst_derivedwireuuid PRIVATE + "QET_TEST_BINARY_PATH=\"$\"" + "QET_EXAMPLES_DIR=\"${QET_DIR}/examples\"") + +# 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_derivedwireuuid.cpp b/tests/qttest/tst_derivedwireuuid.cpp new file mode 100644 index 000000000..7bfc78b04 --- /dev/null +++ b/tests/qttest/tst_derivedwireuuid.cpp @@ -0,0 +1,307 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +// A wire saved without a uuid must get one that is the same on every load, +// is written on save, and survives the edits people make before that save: +// inserting a folio, saving the wires in another order, drawing the wire +// the other way round. Until this was fixed it got a random uuid on every +// load, never saved (#754). +// +// Runs the real binary (--resave) on two fixtures -- one whose wires name +// their ends by symbol and terminal uuid, one older file naming them by +// terminal number -- and reads the wires' 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 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; +} + +QList wires(const QDomElement &diagram) +{ + QList out; + const QDomElement block = diagram.firstChildElement(QStringLiteral("conductors")); + for (QDomElement c = block.firstChildElement(QStringLiteral("conductor")); + !c.isNull(); c = c.nextSiblingElement(QStringLiteral("conductor"))) + out << c; + return out; +} + +// Every wire's uuid in the file, sorted: equal lists mean every wire kept +// its uuid. +QStringList wireUuids(const QDomDocument &doc) +{ + QStringList out; + for (const QDomElement &d : diagrams(doc)) + for (const QDomElement &c : wires(d)) + out << c.attribute(QStringLiteral("uuid")); + out.sort(); + return out; +} + +// Which wire carries which uuid: each wire keyed by its two ends as the +// saved file names them, sorted. An older file names an end by terminal +// number, resolved here to the symbol and the terminal's place on it. +QMap wireMap(const QDomDocument &doc) +{ + QMap out; + for (const QDomElement &d : diagrams(doc)) { + QHash by_number; + const QDomNodeList symbols = d.firstChildElement(QStringLiteral("elements")) + .elementsByTagName(QStringLiteral("element")); + for (int i = 0; i < symbols.size(); ++i) { + const QDomElement e = symbols.at(i).toElement(); + const QDomNodeList ts = e.elementsByTagName(QStringLiteral("terminal")); + for (int j = 0; j < ts.size(); ++j) { + const QDomElement t = ts.at(j).toElement(); + by_number.insert(t.attribute(QStringLiteral("id")), + e.attribute(QStringLiteral("uuid")) + QLatin1Char('/') + + t.attribute(QStringLiteral("x")) + QLatin1Char(',') + + t.attribute(QStringLiteral("y"))); + } + } + for (const QDomElement &c : wires(d)) { + QStringList ends; + for (const QString n : {QStringLiteral("1"), QStringLiteral("2")}) { + const QString element = c.attribute(QStringLiteral("element") + n); + const QString terminal = c.attribute(QStringLiteral("terminal") + n); + ends << (element.isEmpty() ? by_number.value(terminal, QStringLiteral("?")) + : element + QLatin1Char('/') + terminal); + } + ends.sort(); + out.insert(ends.join(QLatin1Char('|')), c.attribute(QStringLiteral("uuid"))); + } + } + return out; +} + +} // namespace + +class tst_derivedwireuuid : 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(120000) || proc.exitCode() != 0) return {}; + return load(out); + } + + // The fixture as an older QElectroTech saved it: wires without uuids. + QDomDocument fixture() + { + QFETCH(QString, path); + QDomDocument doc = load(path); + for (const QDomElement &d : diagrams(doc)) + for (QDomElement c : wires(d)) + c.removeAttribute(QStringLiteral("uuid")); + return doc; + } + + // The saved uuids of the unedited fixture, checked for sanity. + QStringList reference() + { + QFETCH(int, count); + const QStringList ids = wireUuids(resave(fixture())); + if (ids.size() != count) return {}; + for (const QString &u : ids) + if (QUuid(u).isNull()) return {}; + return ids; + } + + void fixtures() + { + QTest::addColumn("path"); + QTest::addColumn("count"); + QTest::newRow("ends by uuid") + << QFINDTESTDATA("fixtures/qet_bug_repro_resaved.qet") << 7; + QTest::newRow("ends by terminal number (older file)") + << QStringLiteral(QET_EXAMPLES_DIR "/tremie_vibrante.qet") << 77; + } + +private slots: + void initTestCase() + { + QVERIFY(m_dir.isValid()); + QVERIFY(QFile::exists(QStringLiteral(QET_TEST_BINARY_PATH))); + } + + void savedAndUnique_data() { fixtures(); } + void savedAndUnique() + { + QFETCH(int, count); + const QStringList ids = reference(); + QCOMPARE(ids.size(), count); // every wire has one + QCOMPARE(QSet(ids.begin(), ids.end()).size(), count); // all different + QCOMPARE(wireMap(resave(fixture())).size(), count); // the test's wire key is unique too + } + + void sameOnEveryLoad_data() { fixtures(); } + void sameOnEveryLoad() + { + const QStringList a = reference(); + QVERIFY(!a.isEmpty()); + QCOMPARE(wireUuids(resave(fixture())), a); + } + + void savedUuidIsReadBack_data() { fixtures(); } + void savedUuidIsReadBack() + { + const QDomDocument once = resave(fixture()); + QVERIFY(!wireUuids(once).isEmpty()); + QCOMPARE(wireUuids(resave(once)), wireUuids(once)); + } + + void insertingAFolioChangesNothing_data() { fixtures(); } + void insertingAFolioChangesNothing() + { + QVERIFY(!reference().isEmpty()); + const QMap before = wireMap(resave(fixture())); + QDomDocument doc = fixture(); + const QDomElement first = diagrams(doc).first(); + QDomElement blank = first.cloneNode(false).toElement(); + blank.setAttribute(QStringLiteral("title"), QStringLiteral("new")); + blank.removeAttribute(QStringLiteral("uuid")); + blank.appendChild(doc.createElement(QStringLiteral("elements"))); + doc.documentElement().insertBefore(blank, first); + QCOMPARE(wireMap(resave(doc)), before); + } + + void wireOrderDoesNotMatter_data() { fixtures(); } + void wireOrderDoesNotMatter() + { + QVERIFY(!reference().isEmpty()); + const QMap before = wireMap(resave(fixture())); + QDomDocument doc = fixture(); + for (const QDomElement &d : diagrams(doc)) { + QDomElement block = d.firstChildElement(QStringLiteral("conductors")); + const QList ws = wires(d); + for (const QDomElement &w : ws) + block.removeChild(w); + for (auto it = ws.crbegin(); it != ws.crend(); ++it) // reversed + block.appendChild(*it); + } + QCOMPARE(wireMap(resave(doc)), before); + } + + void directionDoesNotMatter_data() { fixtures(); } + void directionDoesNotMatter() + { + QVERIFY(!reference().isEmpty()); + const QMap before = wireMap(resave(fixture())); + QDomDocument doc = fixture(); + static const QRegularExpression pair( + QStringLiteral("^(element|terminal|terminalname)([12])(.*)$")); + for (const QDomElement &d : diagrams(doc)) { + for (QDomElement w : wires(d)) { + const QDomNamedNodeMap attrs = w.attributes(); + QHash swapped; + for (int i = 0; i < attrs.size(); ++i) { + const QString name = attrs.item(i).nodeName(); + const auto m = pair.match(name); + if (m.hasMatch()) + swapped.insert(m.captured(1) + (m.captured(2) == QLatin1String("1") + ? QStringLiteral("2") : QStringLiteral("1")) + + m.captured(3), + attrs.item(i).nodeValue()); + } + for (auto it = swapped.cbegin(); it != swapped.cend(); ++it) + w.setAttribute(it.key(), it.value()); + } + } + QCOMPARE(wireMap(resave(doc)), before); + } + + // A wire keeps its uuid once saved, even when re-connected. A wire saved + // without a uuid that later turns up on the ends it left (a hand edit, an + // older version, another tool) would derive the same uuid: it must get + // another one instead. + void newcomerOnAReconnectedWiresEndsGetsAnotherUuid_data() { fixtures(); } + void newcomerOnAReconnectedWiresEndsGetsAnotherUuid() + { + QFETCH(int, count); + QDomDocument doc = resave(fixture()); + QVERIFY(!wireUuids(doc).isEmpty()); + const QList ws = wires(diagrams(doc).first()); + QVERIFY(ws.size() >= 2); + QDomElement x = ws.at(0); + const QDomElement y = ws.at(1); + QDomElement newcomer = x.cloneNode(true).toElement(); // x's old ends + newcomer.removeAttribute(QStringLiteral("uuid")); + // re-connect x's second end to y's second end, keeping x's uuid + static const QRegularExpression end2(QStringLiteral("^(element|terminal|terminalname)2")); + const QDomNamedNodeMap attrs = y.attributes(); + for (int i = 0; i < attrs.size(); ++i) + if (end2.match(attrs.item(i).nodeName()).hasMatch()) + x.setAttribute(attrs.item(i).nodeName(), attrs.item(i).nodeValue()); + x.parentNode().appendChild(newcomer); + + const QStringList after = wireUuids(resave(doc)); + QCOMPARE(after.size(), count + 1); + QCOMPARE(QSet(after.begin(), after.end()).size(), count + 1); + } + + // QElectroTech refuses a second wire between two terminals already joined + // (Terminal::canBeLinkedTo()), so two wires on one folio never share both + // ends. A file that has one anyway must still load to the same uuids. + void doubledWireIsDroppedAndOthersKeepTheirs_data() { fixtures(); } + void doubledWireIsDroppedAndOthersKeepTheirs() + { + QVERIFY(!reference().isEmpty()); + const QMap before = wireMap(resave(fixture())); + QDomDocument doc = fixture(); + QDomElement w = wires(diagrams(doc).first()).first(); + w.parentNode().appendChild(w.cloneNode(true)); + QCOMPARE(wireMap(resave(doc)), before); + } +}; + +QTEST_APPLESS_MAIN(tst_derivedwireuuid) + +#include "tst_derivedwireuuid.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" diff --git a/tests/qttest/tst_scriptconductoruuid.cpp b/tests/qttest/tst_scriptconductoruuid.cpp new file mode 100644 index 000000000..27792ab70 --- /dev/null +++ b/tests/qttest/tst_scriptconductoruuid.cpp @@ -0,0 +1,104 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +// qet.conductorUuids(folio) and qet.conductorEnds(folio, uuid): a script can +// list a folio's conductors by uuid and find where each one runs, in the +// "{element} terminal N" form conductors() prints and the conductor calls +// take. Runs a script through the real binary's --run. +class tst_scriptconductoruuid : public QObject +{ + Q_OBJECT + + QTemporaryDir m_dir; + + // Run @p script on the fixture in a sandbox of its own and return the + // JSON object it logged. + QJsonObject run(const QString &script) + { + const QString path = m_dir.filePath(QStringLiteral("probe.js")); + const QString home = m_dir.filePath(QStringLiteral("home")); + QDir().mkpath(home); + QFile f(path); + if (!f.open(QIODevice::WriteOnly)) return {}; + f.write(script.toUtf8()); + f.close(); + + QProcessEnvironment env = QProcessEnvironment::systemEnvironment(); + env.insert(QStringLiteral("QT_QPA_PLATFORM"), QStringLiteral("offscreen")); + env.insert(QStringLiteral("QET_ENABLE_SCRIPTING"), QStringLiteral("1")); + 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("--run"), path, + QFINDTESTDATA("fixtures/qet_bug_repro_resaved.qet")}); + if (!proc.waitForFinished(60000)) return {}; + const QString out = QString::fromUtf8(proc.readAllStandardOutput() + + proc.readAllStandardError()); + const QString mark = QStringLiteral("PROBE "); + for (const QString &line : out.split(QLatin1Char('\n'))) { + const int i = line.indexOf(mark); + if (i >= 0) + return QJsonDocument::fromJson(line.mid(i + mark.size()).toUtf8()).object(); + } + return {}; + } + +private slots: + void initTestCase() + { + QVERIFY(m_dir.isValid()); + QVERIFY(QFile::exists(QStringLiteral(QET_TEST_BINARY_PATH))); + } + + void uuidsAndEndsMatchConductors() + { + const QJsonObject r = run(QStringLiteral( + "var uuids = qet.conductorUuids(0);\n" + "var ends = uuids.map(function (u) { return qet.conductorEnds(0, u); });\n" + "qet.log('PROBE ' + JSON.stringify({lines: qet.conductors(0), uuids: uuids, ends: ends,\n" + " unknown: qet.conductorEnds(0, '{00000000-0000-0000-0000-000000000001}'),\n" + " junk: qet.conductorEnds(0, 'not a uuid'),\n" + " badFolio: qet.conductorUuids(99)}));\n")); + QVERIFY2(!r.isEmpty(), "the script logged nothing"); + + const QJsonArray lines = r.value(QStringLiteral("lines")).toArray(); + const QJsonArray uuids = r.value(QStringLiteral("uuids")).toArray(); + const QJsonArray ends = r.value(QStringLiteral("ends")).toArray(); + QCOMPARE(lines.size(), 7); // the fixture's conductors + QCOMPARE(uuids.size(), lines.size()); + QSet distinct; + for (int i = 0; i < uuids.size(); ++i) { + const QString u = uuids.at(i).toString(); + QVERIFY2(!QUuid(u).isNull(), qPrintable(u)); + distinct.insert(u); + // same order as conductors(), and the same two ends it prints + const QJsonArray e = ends.at(i).toArray(); + QCOMPARE(e.size(), 2); + const QString expected = e.at(0).toString() + QStringLiteral(" -- ") + + e.at(1).toString() + QStringLiteral(" : "); + QVERIFY2(lines.at(i).toString().startsWith(expected), + qPrintable(lines.at(i).toString() + QStringLiteral(" | ") + expected)); + } + QCOMPARE(distinct.size(), uuids.size()); + + QVERIFY(r.value(QStringLiteral("unknown")).toArray().isEmpty()); + QVERIFY(r.value(QStringLiteral("junk")).toArray().isEmpty()); + QVERIFY(r.value(QStringLiteral("badFolio")).toArray().isEmpty()); + } +}; + +QTEST_APPLESS_MAIN(tst_scriptconductoruuid) + +#include "tst_scriptconductoruuid.moc"