diff --git a/sources/diagramcontext.cpp b/sources/diagramcontext.cpp index 5137a7bf5..94b8696a2 100644 --- a/sources/diagramcontext.cpp +++ b/sources/diagramcontext.cpp @@ -144,6 +144,27 @@ bool DiagramContext::operator!=(const DiagramContext &dc) const return(!(*this == dc)); } +namespace { +/** + The value as it is saved, and so as it is read back: stray leading and + trailing whitespace around real content trimmed, but not a value that IS + whitespace -- unconditionally trimming an all-whitespace string collapses + it to "", which is silently indistinguishable from a value that was never + set. A title-block custom variable set to a single space -- a workaround + for #973, where an unset variable renders as its own literal placeholder + -- would otherwise vanish on the very next save. + Applied when reading as well as when writing, so that what is in memory + after a load is what the next save writes: a label shown from an + untrimmed value would otherwise keep its spaces on screen and in its + displayed copy until the project was saved and opened again, and a + just-saved project would change on its second save. +*/ +QString storedValue(const QString &raw) +{ + return raw.trimmed().isEmpty() ? raw : raw.trimmed(); +} +} // namespace + /** Export this context properties under the \a e XML element, using tags named \a tag_name (defaults to "property"). @@ -161,15 +182,7 @@ void DiagramContext::toXml(QDomElement &e, const QString &tag_name) const property.removeAttribute("name"); property.setAttribute("show", m_content_show[key]); property.setAttribute("name", key); - // Trim stray leading/trailing whitespace around real content, but - // not a value that IS whitespace: unconditionally trimming an - // all-whitespace string collapses it to "", which is silently - // indistinguishable from a value that was never set. A title-block - // custom variable set to a single space -- a workaround for #973, - // where an unset variable renders as its own literal placeholder -- - // would otherwise vanish on the very next save. - const QString stored = raw.trimmed().isEmpty() ? raw : raw.trimmed(); - QDomText value = e.ownerDocument().createTextNode(stored); + QDomText value = e.ownerDocument().createTextNode(storedValue(raw)); property.appendChild(value); e.appendChild(property); } @@ -182,7 +195,7 @@ void DiagramContext::toXml(QDomElement &e, const QString &tag_name) const void DiagramContext::fromXml(const QDomElement &e, const QString &tag_name) { foreach (QDomElement property, QET::findInDomElement(e, tag_name)) { if (!property.hasAttribute("name")) continue; - addValue(property.attribute("name"), QVariant(property.text())); + addValue(property.attribute("name"), QVariant(storedValue(property.text()))); m_content_show.insert(property.attribute("name"), property.attribute("show", "1").toInt()); } } @@ -198,7 +211,8 @@ void DiagramContext::fromXml(const pugi::xml_node &dom_element, const QString &t { for(auto node = dom_element.child(tag_name.toStdString().c_str()) ; node ; node = node.next_sibling(tag_name.toStdString().c_str())) { - addValue(node.attribute("name").as_string(), QVariant(node.text().as_string())); + addValue(node.attribute("name").as_string(), + QVariant(storedValue(QString::fromUtf8(node.text().as_string())))); m_content_show.insert(node.attribute("name").as_string(), node.attribute("show").empty()? 1 : node.attribute("show").as_int()); } } diff --git a/sources/qetgraphicsitem/element.cpp b/sources/qetgraphicsitem/element.cpp index fbbc3df5d..eb9dbfbe2 100644 --- a/sources/qetgraphicsitem/element.cpp +++ b/sources/qetgraphicsitem/element.cpp @@ -1033,7 +1033,11 @@ QDomElement Element::toXml( QDomElement infos = document.createElement(QStringLiteral("elementInformations")); m_data.m_informations.toXml(infos, QStringLiteral("elementInformation")); - element.appendChild(infos); + //toXml() skips empty values: an element whose information is + //all empty would otherwise be written an empty block, which the + //next load reads as no information and the next save drops. + if (infos.hasChildNodes()) + element.appendChild(infos); } //Save override properties (For now, only used when the element is a terminal) diff --git a/tests/qttest/CMakeLists.txt b/tests/qttest/CMakeLists.txt index 94283cc09..525ccb8cd 100644 --- a/tests/qttest/CMakeLists.txt +++ b/tests/qttest/CMakeLists.txt @@ -328,3 +328,16 @@ target_include_directories(tst_conductorselfretrace PRIVATE ${QET_DIR}/sources) target_link_libraries(tst_conductorselfretrace PRIVATE Qt::Test) target_compile_definitions(tst_conductorselfretrace PRIVATE "QET_TEST_BINARY_PATH=\"$\"") + +# Saving a project that was just saved changes nothing: runs the real +# binary's --resave twice on examples/Projet_vierge.qet (empty information +# values) and examples/m_000.qet (information values with stray spaces). +add_executable( + tst_resaveunchanged + tst_resaveunchanged.cpp) +add_test(NAME tst_resaveunchanged COMMAND tst_resaveunchanged) +add_dependencies(tst_resaveunchanged qelectrotech) +target_link_libraries(tst_resaveunchanged PRIVATE Qt::Test) +target_compile_definitions(tst_resaveunchanged PRIVATE + "QET_TEST_BINARY_PATH=\"$\"" + "QET_EXAMPLES_DIR=\"${QET_DIR}/examples\"") diff --git a/tests/qttest/tst_resaveunchanged.cpp b/tests/qttest/tst_resaveunchanged.cpp new file mode 100644 index 000000000..aff0fe360 --- /dev/null +++ b/tests/qttest/tst_resaveunchanged.cpp @@ -0,0 +1,81 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +#include + +#include +#include +#include +#include +#include + +// Saving a project that was just saved must change nothing. Two things +// made the second save differ from the first, both cleanup done on save +// but not on load: +// - symbol information whose values were all empty was written as an +// empty block, which the next load read as no +// information and the next save dropped (Projet_vierge.qet); +// - information values were trimmed on save but not on load, so a label +// with stray spaces kept them in its displayed copy until the project +// was opened again (m_000.qet). +// Runs the real binary's --resave twice on each example. +class tst_resaveunchanged : public QObject +{ + Q_OBJECT + + QTemporaryDir m_dir; + int m_run = 0; + + // --resave @p in to a new file, in a sandbox of its own (so a running + // QElectroTech cannot answer instead); returns the new file's path. + QString resave(const QString &in) + { + const QString out = m_dir.filePath(QStringLiteral("out%1.qet").arg(m_run)); + const QString home = m_dir.filePath(QStringLiteral("home%1").arg(m_run++)); + QDir().mkpath(home); + QProcessEnvironment env = QProcessEnvironment::systemEnvironment(); + env.insert(QStringLiteral("QT_QPA_PLATFORM"), QStringLiteral("offscreen")); + env.insert(QStringLiteral("HOME"), home); + env.insert(QStringLiteral("XDG_CONFIG_HOME"), home + QStringLiteral("/config")); + env.insert(QStringLiteral("XDG_DATA_HOME"), home + QStringLiteral("/data")); + QProcess proc; + proc.setProcessEnvironment(env); + proc.start(QStringLiteral(QET_TEST_BINARY_PATH), {QStringLiteral("--resave"), in, out}); + if (!proc.waitForFinished(120000) || proc.exitCode() != 0) return {}; + return out; + } + + static QByteArray read(const QString &path) + { + QFile f(path); + return f.open(QIODevice::ReadOnly) ? f.readAll() : QByteArray(); + } + +private slots: + void initTestCase() + { + QVERIFY(m_dir.isValid()); + QVERIFY(QFile::exists(QStringLiteral(QET_TEST_BINARY_PATH))); + } + + void secondSaveChangesNothing_data() + { + QTest::addColumn("project"); + QTest::newRow("empty information values") << QStringLiteral("Projet_vierge.qet"); + QTest::newRow("information values with stray spaces") << QStringLiteral("m_000.qet"); + } + + void secondSaveChangesNothing() + { + QFETCH(QString, project); + const QString first = resave(QStringLiteral(QET_EXAMPLES_DIR "/") + project); + QVERIFY2(!first.isEmpty(), "first --resave failed"); + const QString second = resave(first); + QVERIFY2(!second.isEmpty(), "second --resave failed"); + const QByteArray a = read(first), b = read(second); + QVERIFY(!a.isEmpty()); + QVERIFY2(a == b, "the second save changed the file"); + } +}; + +QTEST_APPLESS_MAIN(tst_resaveunchanged) + +#include "tst_resaveunchanged.moc"