Skip to content

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

Merged
merged 14 commits into from
Apr 28, 2025

Conversation

hartwork
Copy link
Member

@hartwork hartwork commented Apr 26, 2025

Fixes #163

Please note that:

  • I have not migrated off of class QRegExp because successor QRegularExpression 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.
  • While I have applied care here, true C++ is not my day-to-day language so this should be reviewed carefully before merging. Thanks in advance!
  • If this gets merged, please do not squash-merge but preserve the commit cut lines that have been chosen carefully. Thank you!

@a17r
Copy link

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.

@hartwork
Copy link
Member Author

@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.

@tnyblom
Copy link
Contributor

tnyblom commented Apr 28, 2025

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
Copy link

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.

@hartwork
Copy link
Member Author

hartwork commented Apr 28, 2025

@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.

@hartwork
Copy link
Member Author

@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
Copy link

@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?

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...

@hartwork
Copy link
Member Author

hartwork commented Apr 28, 2025

@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…

@hartwork hartwork merged commit e0fff94 into master Apr 28, 2025
10 checks passed
@hartwork hartwork deleted the qt-5-and-qt-6 branch April 28, 2025 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Distros are removing support for Qt 5, we need to add support for Qt 6 to not be dropped
4 participants