-
Notifications
You must be signed in to change notification settings - Fork 103
Support both Qt 5 and Qt 6 at the same time (fixes #163) #165
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
Conversation
a17r
commented
Apr 27, 2025
For unconditionally using Qt::endl
you'll have to raise minimum Qt5 version to at least 5.14. Did not check the other stuff, but likely better 5.15.
4ccb673
to
02617df
Compare
02617df
to
c80d82a
Compare
@a17r thanks for the review and for bringing this up. I confirm 5.14 to be needed for...
...and made the QMake project file reject Qt <5.14.0 now. It seems like the only changes not guarded behind >=6.0.0 checks so I'm leaning towards 5.14 rather than 5.15 so far. Pushed.
I've not used C++ or Qt in quite a few years, but with that caveat out of the way... Looks good to me.
svuorela
commented
Apr 28, 2025
I think it could be simpler in a couple of steps.
First build with qt5.15 and get rid of all deprecated warnings. Then port to Qt6.
At least all of the QMultiMap stuff should be equally good in qt5.
c80d82a
to
6333d1b
Compare
@tnyblom @svuorela thanks for the review!
@svuorela I confirm that (1) not supporting both Qt 5 and Qt 6 at the same time would be simpler and (2) that Qt 5 does have QMultiMap
... but it does not have QMultiMapIterator
and it would need a closer look how to mimic/replace it. My goal here was to keep Qt 5 in the boat for now and to add support for Qt 6 with minimum changes and have at least one release supporting both before cutting support for Qt 5 off.
@svuorela PS: With regard to QMultiMap
let's make a follow-up pull request to either (a) use QMultiMap
without QMultiMapIterator
for both Qt 5 and 6 or (b) to drop support for Qt 5 altogether. (b) I can do, for (a) I'd be happy about contributions. What do you think?
svuorela
commented
Apr 28, 2025
@svuorela PS: With regard to
QMultiMap
let's make a follow-up pull request to either (a) useQMultiMap
withoutQMultiMapIterator
for both Qt 5 and 6 or (b) to drop support for Qt 5 altogether. (b) I can do, for (a) I'd be happy about contributions. What do you think?
I'm just a driveby reviewer, so don't get too much riled up about my ideas.
Maybe just a
#if qt6 using MapIterator = QMultiMapIterator; #else using MapIterator = QMapIterator; #endif
and then just use MapIterator everywhere...
@svuorela either I misunderstand the idea or you want to use QMapIterator
with QMultiMap
in case of Qt 6 but they do not seem to be compatible. I'll go ahead and merge and we can then reduce complexity...
Uh oh!
There was an error while loading. Please reload this page.
Fixes #163
Please note that:
QRegExp
because successorQRegularExpression
has a different pattern syntax and so backwards compatibility would be broken e.g. with regard to existing rules files. That migration should probably be a distinct future pull request and come with a bump in major version to signal its breaking nature.