-
Notifications
You must be signed in to change notification settings - Fork 296
Migrate to PySide6 #1601
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
Migrate to PySide6 #1601
Conversation
Moved to Qt6 scoped enums Enabled PyQt6 pyuic
|
Thank you so much for working on this! This migration is incredibly important for the maintainability of plover. I'm happy to test this the next days on Windows, Linux, and macOS. Let me know if there's anything else I can do to help. |
|
@mkrnr Any testing you can do would be very helpful. Post any error messages or not-working features here. If there are any plugins you use that you can test that would be useful, although I expect only non-graphical plugins have a chance of working without updates. The packaging systems need fixing too (see all the red in the CI right now...) . I'll take a look into that over the next few days/weeks but if anyone knows how all the packaging scripts work then help would be appreciated. |
|
Launches successfully on Apple Silicon Macbook Air (M2 chip). Qt6 takes system theme (Dark) too, neat! |
|
Here are some tests on Windows 11 64 bit using a Georgi: tox r -e launchall built-in functionality works flawlessly:
Warnings during start, maybe the one regarding tox r -e testonly failures are in: This fails on the master as well and is likely fixed by #1599. tox r -e plugins_installExecuted successfully. Is there a way to test this at this point? Because when I then run tox r -e plugins_install -- C:\Users\mkoer\git\plover_regenpfeiferExecuted successfully. Don't know how to test this, see above. tox r -e packaging_checkstox r -e release_prepare |
|
Thanks for running the tests, that's great to know this works for other people.
Don't think these warnings happened for Qt5 - I'll look at what's causing these messages
You might be able to launch plover directly using the environment created by tox. Enter the environment (something like I'm going to look at packaging this, then hopefully we'll be able to share this and get some beta testers for a Qt6 Plover release. |
|
I'll give that a try next days, thanks! We'll also have to look into updating plover_plugins_manager to Qt6 since that's such an integral part of plover. Did you look into that already by any chance? |
bed0d2b to
d7065ba
Compare
Switch builds to PyQt6 Include resources folder in distribution Remove extra MANIFEST.in entries to silence warnings Pin PyQt6-Qt6 - https://github.com/googlefonts/fontra-pak/pull/27 Add pyqt6rc to test/build requirements
d7065ba to
c428339
Compare
|
I've had a quick look at the plugin manager - there are a few small changes needed there for Qt6 but shouldn't be anything too major. We'll need to update both this project and the plugin manager together though as they're mutually dependent. |
edec04b to
327a818
Compare
|
Awesome to see your progress on this, now with the plugin manager migration! I'd like to help you testing this but when I launch with tox ( |
|
Most of my recent testing has been through the CI system in GitHub, just fixing errors and getting the builds working. The CI versions have the plugin manager bundled (which was causing build failures using a non-patched Qt5 plugin manager version) - you can download these as build artefacts. Seems to work for the Linux AppImage, but the Windows exe doesn't yet run. I've yet to sort out how to use the plugin manager successfully when launching locally from tox. Getting local builds to work is not easy - you have to have almost exactly the same setup (including library versions) that the build scripts need else you'll get a bunch of unhelpful error messages from pip. I've also run into issues where some of the download sources for build scripts are rate limited so stop working after a few attempts. Honestly the build system needs work to be simplified, it's a little unmanageable right now. |
This reverts commit a299989.
|
do you need any help with this? eager to get a better working version of plover. |
Thanks a lot @appetrosyan for offering support! Since the notification issues are fixed now, the most important help would be thorough testing and reviewing the changes. That's what I'll focus on over the week or so. Other changes like bumping python and other dependencies should come in a separate PR. Feel free to reach out in the plover-dev channel on the plover discord! |
This reverts commit 1443ee9.
|
Merging this PR without review. Several people gave positive feedback after testing though. |
|
It's such a massive change though. I may report any breaking API change later but it may take a while (given I have several custom modifications/patches to Plover that need to be ported). |
Summary of changes
To be handled in separate PRs:
registry_v2.jsonin plover_plugins_registryin case we want to start removing migrated plugins from the
registry.jsonfor plover 4Pull Request Checklist
Progress
5.0.0.dev1as this is a breaking changeTests
plover-combo, see here) as a proof of concept and was able to run inInstaller Files
If you want to test the installer files yourself, you can check the GitHub Actions runs like this one. In some cases, not all installer files are included since not all steps in the GitHub actions were executed. In that case, check the previous runs.
After Merge