Merge pull request #862 from ispyisail/fix/bugtracker-108-junction-dot-width

Fix bugtracker #108: the junction dot vanishes on a wide conductor

ELEMENTS.LICENSE

@@ -1,3 +1,20 @@
+[ca]
+La col·lecció d'elements que s'inclou amb QElectrotech es proporciona tal com és 
+i sense cap garantia que sigui adequada per al vostre ús o que funcioni correctament. 
+L'ús, la modificació i la integració d'aquests elements en esquemes elèctrics 
+estan permesos sense restriccions, independentment de la llicència final que regeixi 
+els esquemes.
+No es permet utilitzar aquest programari ni cap fitxer associat
+com a dades de mostra per crear models d'aprenentatge automàtic.
+
+Si redistribuïu la totalitat o una part de la col·lecció QElectroTech, 
+amb o sense modificacions, fora d'un esquema elèctric, heu de complir 
+les condicions de la llicència CC-BY:
+Aquesta obra està subjecta a la llicència Reconeixement 3.0,
+disponible en línia a http://creativecommons.org/licenses/by/3.0/ o bé
+sol·licitant-la per correu a Creative Commons, 171 Second Street, Suite 300, San Francisco,
+Califòrnia 94105, EUA.
+
 [en]
 The elements collection provided along with QElectroTech is provided as is and
 without any warranty of fitness for your purpose or working.
@@ -96,19 +113,6 @@ Para ver una copia de esta licencia, visite
 http://creativecommons.org/licenses/by/3.0/ o envie una carta a Creative
 Commons, 171 Second Street, Suite 300, San Francisco, California 94105, USA.
 
-[ca]
-La col·lecció de símbols QElectrotech és distribuïda tal qual i sense cap
-garantia d'idoneïtat d'ús ni de funcionament.
-Es permet incondicionalment, amb independència de la llicència final, emprar,
-editar, i incloure aquests símbols en esquemes elèctrics.
-Si vostè redistribueix una part de la col·lecció de QElectrotech o tota ella,
-amb condicions o sense, separadament d'un esquema elèctric, haurà de respectar
-les condicions de la llicència CC-BY:
-Aquesta obra es troba sota una llicència Reconeixement 3.0 de Creative Commons.
-Per veure una còpia d'aquesta llicència visiti
-http://creativecommons.org/licenses/by/3.0/ o enviï una carta a Creative
-Commons, 171 Second Street, Suite 300, San Francisco, California 94105, 
-
 [cs]
 Sbírka prvků poskytovaná společně s QElectroTechem je poskytována tak, jak je,
 bez záruky nebo vhodnosti pro váš účal nebo práci.

sources/qet.cpp

@@ -546,12 +546,40 @@ QString QET::joinWithSpaces(const QStringList &string_list) {
 QStringList QET::splitWithSpaces(const QString &string) {
 	// les chaines sont separees par des espaces non echappes
 	// = avec un nombre nul ou pair de backslashes devant
-
-	QStringList escaped_strings = string.split(QRegularExpression("[^\\]?(?:\\\\)* "),Qt::SkipEmptyParts);
-
+	//
+	// This was a QRegularExpression("[^\\]?(?:\\\\)* ") split, which never
+	// worked: "[^\]" opens a character class whose "\]" is an escaped
+	// bracket, so the class is never closed and the pattern is invalid.
+	// QRegularExpression::isValid() was false, QString::split() warned
+	// "invalid QRegularExpression object" and returned an EMPTY list for
+	// every input -- so a second instance's file arguments were always
+	// dropped (bugtracker #248), not just ones containing spaces.
+	//
+	// A correct pattern is not expressible here either: the separator is a
+	// space preceded by an even-length run of backslashes, and PCRE2 has no
+	// variable-length lookbehind. Scanning explicitly is both correct and
+	// easier to read than the alternatives.
 	QStringList returned_list;
-	foreach(QString escaped_string, escaped_strings) {
-		returned_list << QET::unescapeSpaces(escaped_string);
+	QString current;
+	int backslashes = 0;
+	for (const QChar &c : string) {
+		if (c == QLatin1Char('\\')) {
+			++backslashes;
+			current += c;
+			continue;
+		}
+		if (c == QLatin1Char(' ') && backslashes % 2 == 0) {
+			if (!current.isEmpty()) {
+				returned_list << QET::unescapeSpaces(current);
+			}
+			current.clear();
+		} else {
+			current += c;
+		}
+		backslashes = 0;
+	}
+	if (!current.isEmpty()) {
+		returned_list << QET::unescapeSpaces(current);
 	}
 	return(returned_list);
 }

sources/qetapp.cpp

@@ -1659,7 +1659,31 @@ void QETApp::receiveMessage(int instanceId, QByteArray message)
 	{
 		QString my_message(str.mid(20));
 		QStringList args_list = QET::splitWithSpaces(my_message);
-		openFiles(QETArguments(args_list));
+
+		// Deferred, not called directly.
+		//
+		// This slot runs inside SingleApplication's readyRead handling:
+		// SingleApplicationPrivate::slotDataAvailable() emits
+		// receivedMessage() synchronously from the socket's readyRead
+		// lambda. openFiles() then loads a project -- seconds of work on
+		// a large one -- and openAndAddProject() puts up a modal
+		// BackupDialog, whose exec() runs a nested event loop while the
+		// socket handler is still on the stack.
+		//
+		// During that nested loop the secondary instance exits, the
+		// connection closes and the QLocalSocket is deleted. When the
+		// dialog is dismissed and the stack unwinds, QMetaObject::
+		// activate() continues emitting on the freed sender and the
+		// process dies. Reported with a backtrace on PR #861;
+		// reproduced on Qt 6.10.2 by dismissing the dialog, which is the
+		// step that makes it fail -- leaving it open never unwinds.
+		//
+		// A zero-timer returns to the event loop first, so the socket
+		// stack is fully unwound before any of this runs.
+		const QETArguments deferred_args{args_list};
+		QTimer::singleShot(0, this, [this, deferred_args]() {
+			openFiles(deferred_args);
+		});
 	}
 }
 

sources/qetgraphicsitem/conductor.cpp

@@ -574,8 +574,23 @@ void Conductor::paint(QPainter *painter, const QStyleOptionGraphicsItem *options
 		painter -> setPen(final_conductor_pen);
 		painter -> setBrush(junction_brush);
 		painter -> setRenderHint(QPainter::Antialiasing, true);
+			// The junction dot has to read as a dot on top of the conductor
+			// that carries it, so it scales with the conductor width instead
+			// of being a fixed 3.0 across: on a wide conductor a 3.0 dot is
+			// narrower than the line and simply disappears (bugtracker #108).
+			//
+			// Floored at the historic 3.0 so nothing changes for the default
+			// width of 1.0 or anything thinner -- only the wide conductors
+			// the report is about are affected. m_properties.cond_size is
+			// used rather than the pen, whose width is inflated by 4 while
+			// the mouse is over the conductor.
+		const qreal junction_diameter = qMax(3.0, 3.0 * m_properties.cond_size);
+		const qreal junction_radius = junction_diameter / 2.0;
 		foreach(QPointF point, junctions_list) {
-			painter -> drawEllipse(QRectF(point.x() - 1.5, point.y() - 1.5, 3.0, 3.0));
+			painter -> drawEllipse(QRectF(point.x() - junction_radius,
+						      point.y() - junction_radius,
+						      junction_diameter,
+						      junction_diameter));
 		}
 	}
 

tests/qttest/CMakeLists.txt

@@ -100,3 +100,16 @@ add_executable(
 add_test(NAME tst_smart_device COMMAND tst_smart_device)
 target_include_directories(tst_smart_device PRIVATE ${QET_DIR}/sources)
 target_link_libraries(tst_smart_device PRIVATE Qt::Test Qt::Sql)
+
+# qet.cpp carries the SingleApplication argument wire format
+# (joinWithSpaces/splitWithSpaces); it pulls in qeticons and shortcutmanager,
+# so those are compiled alongside rather than linking the whole application.
+add_executable(
+  tst_qetstrings
+  tst_qetstrings.cpp
+  ${QET_DIR}/sources/qet.cpp
+  ${QET_DIR}/sources/qeticons.cpp
+  ${QET_DIR}/sources/shortcutmanager.cpp)
+add_test(NAME tst_qetstrings COMMAND tst_qetstrings)
+target_include_directories(tst_qetstrings PRIVATE ${QET_DIR}/sources)
+target_link_libraries(tst_qetstrings PRIVATE Qt::Test Qt::Widgets Qt::Xml)

tests/qttest/tst_qetstrings.cpp

@@ -0,0 +1,54 @@
+#include <QtTest>
+
+#include "qet.h"
+
+/**
+	QET::joinWithSpaces() / QET::splitWithSpaces() are the wire format for the
+	SingleApplication message a secondary instance sends to the running one
+	(main.cpp: "launched-with-args: " + joinWithSpaces(...), received by
+	QETApp::receiveMessage()). If the round trip loses arguments, opening a
+	file while QET is already running silently does nothing.
+
+	splitWithSpaces() used to split on QRegularExpression("[^\\]?(?:\\\\)* "),
+	which is not a valid pattern: "[^\\]" opens a character class whose "\\]"
+	is an escaped bracket, so the class never closes. QRegularExpression
+	reported isValid() == false and QString::split() returned an empty list for
+	every input -- bugtracker #248.
+*/
+class tst_qetstrings : public QObject
+{
+	Q_OBJECT
+
+private slots:
+	void roundTrips_data()
+	{
+		QTest::addColumn<QStringList>("input");
+
+		QTest::newRow("single plain")        << QStringList{"one.qet"};
+		QTest::newRow("two plain")           << QStringList{"one.qet", "two.qet"};
+		QTest::newRow("space in name")       << QStringList{"my file.qet"};
+		QTest::newRow("space then plain")    << QStringList{"my file.qet", "other.qet"};
+		QTest::newRow("spaces in path")      << QStringList{"/home/a b/c d.qet", "/tmp/x.qet"};
+		QTest::newRow("backslash in name")   << QStringList{"back\\slash.qet"};
+		QTest::newRow("trailing backslash")  << QStringList{"trailing\\"};
+		QTest::newRow("mixed")               << QStringList{"a b", "c\\d", "e"};
+	}
+
+	/// What the IPC actually needs: whatever went in comes back out.
+	void roundTrips()
+	{
+		QFETCH(QStringList, input);
+		QCOMPARE(QET::splitWithSpaces(QET::joinWithSpaces(input)), input);
+	}
+
+	/// The specific regression: the old implementation returned an empty list
+	/// for every input, so this passed nothing on to openFiles().
+	void splitIsNotEmptyForPlainArguments()
+	{
+		QVERIFY(!QET::splitWithSpaces(QStringLiteral("one.qet")).isEmpty());
+		QCOMPARE(QET::splitWithSpaces(QStringLiteral("a.qet b.qet")).count(), 2);
+	}
+};
+
+QTEST_APPLESS_MAIN(tst_qetstrings)
+#include "tst_qetstrings.moc"