-
Notifications
You must be signed in to change notification settings - Fork 334
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expose enums to the Qt meta-object system #207
Conversation
Is the dependency on Qt 5.8+ acceptable? |
@@ -52,6 +52,7 @@ QT_FORWARD_DECLARE_CLASS(QSslError) | |||
#endif // QT_NO_SSL | |||
|
|||
namespace QMQTT { | |||
Q_MQTT_EXPORT Q_NAMESPACE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since Qt 5.14, there's also Q_NAMESPACE_EXPORT().
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 What does @ejvr think? |
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 ***@***.***(https://github.com/ejvr) think?
—
You are receiving this because you were mentioned.
Reply to this email directly, [view it on GitHub](#207 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AENEMJNYKOILWPYNWSDI6MTSEOT7PANCNFSM4Q226L5Q).
|
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, and Q_NAMESPACE_EXPORT() was introduced in Qt 5.14.
Qt 5.6:
Qt 5.9:
Qt 5.12:
Qt 5.14:
|
I wanted to do:
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:
|
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. |
This allows e.g. conversions between enum values and key names:
Furthermore, this lets QDebug print out descriptive names instead of
plain int values:
NOTE: Q_NAMESPACE & Q_ENUM_NS were introduced in Qt 5.8.