Expose enums to the Qt meta-object system

[jpnurmi]

This allows e.g. conversions between enum values and key names:

QMetaEnum metaEnum = QMetaEnum::fromType<QMQTT::ClientError>();
qDebug() << metaEnum.keyToValue("SocketTimeoutError"); // "6"

Furthermore, this lets QDebug print out descriptive names instead of plain int values:

qDebug() << QMQTT::STATE_CONNECTING; // "1" vs. "QMQTT::STATE_CONNECTING"

NOTE: Q_NAMESPACE & Q_ENUM_NS were introduced in Qt 5.8.

[jpnurmi]

Is the dependency on Qt 5.8+ acceptable?

[mwallnoefer]

Is the dependency on Qt 5.8+ acceptable?

Yes, good point. I think that currently we achieve compatibility back to Qt 5.5-5.6. Qt 5.7 for instance is the default version shipped with Ubuntu 16.04 LTS (and derivatives) which is still not end-of-life. On the other hand I guess that it shouldn't be that hard to introduce some no-op fill macros for Q_NAMESPACE and Q_ENUM_NS at the beginning of qmqtt_client.h.

What does @ejvr think?

[ejvr]

Hmm, I agree that the QT 5.8+ dependency could cause problems with other users. The strong point of QMQTT is that it works with old QT versions.

However, I like the change: it could be useful when trouble shooting. What about surrounding the proposed changes with something like this?

if QT_VERSION >= 0x050800

...

endif

(not sure this will work, did not try)

Is the dependency on Qt 5.8+ acceptable?

Yes, good point. I think that we currently use to be compatible up to Qt 5.5-5.6. Qt 5.7 for instance is the default version shipped with Ubuntu 16.04 LTS-like OS which is still not end-of-life. On the other hand I guess that it shouldn't be that hard to introduce some no-op fill macros for Q_NAMESPACE and Q_ENUM_NS at the beginning of qmqtt_client.h.

What does @ejvr think?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[jpnurmi]

Qt 5.6:

qDebug() << QT_VERSION_STR << QMQTT::STATE_CONNECTING; // no Q_ENUM_NS so can't use QMetaEnum::fromType() either
// "5.6.2 1"

Qt 5.9:

qDebug() << QT_VERSION_STR << QMQTT::STATE_CONNECTING << QMetaEnum::fromType<QMQTT::ClientError>().keyToValue("SocketTimeoutError");
// "5.9.9 QMQTT::ConnectionState(STATE_CONNECTING) 6"

Qt 5.12:

qDebug() << QT_VERSION_STR << QMQTT::STATE_CONNECTING << QMetaEnum::fromType<QMQTT::ClientError>().keyToValue("SocketTimeoutError");
// "5.12.8 QMQTT::STATE_CONNECTING 6"

Qt 5.14:

qDebug() << QT_VERSION_STR << QMQTT::STATE_CONNECTING << QMetaEnum::fromType<QMQTT::ClientError>().keyToValue("SocketTimeoutError");
// "5.14.2 QMQTT::STATE_CONNECTING 6"

[jpnurmi]

I wanted to do:

#ifndef Q_NAMESPACE_EXPORT
#ifdef Q_NAMESPACE
#define Q_NAMESPACE_EXPORT(x) x Q_NAMESPACE
#else
#define Q_NAMESPACE_EXPORT(x)
#endif // Q_NAMESPACE
#endif // Q_NAMESPACE_EXPORT

namespace QMQTT {
Q_NAMESPACE_EXPORT(Q_MQTT_EXPORT)
// ...
}

because then you could simply just throw out the Q_NAMESPACE_EXPORT macro definition whenever you're ready to depend on Qt 5.14+ in the future, but unfortunately, moc was not happy with this approach:

qmqtt_client.h:66: Error: Namespace declaration lacks Q_NAMESPACE macro.

[ejvr]

I guess this is a bit tricky, because qmake probably scans for the Q_NAMESPACE macro itself when parsing the header file. It does not call the preprocessor, so when compiler with older QT versions, moc.exe does not see Q_NAMESPACE.

Anyway, I like the current patch. Assuming it compiles with older version of QT, I'm in favour of accepting it.

EDIT: the patch compiles with QT 5.11.3, 5.15.1 and 5.7.1.