search

freedomofpress / dangerzone

Take potentially dangerous PDFs, office documents, or images and convert them to safe PDFs
https://dangerzone.rocks/
GNU Affero General Public License v3.0
3.58k stars 168 forks source link

Use `exec` instead of `exec_` for Qt dialogs #795

Closed naglis closed 4 months ago

naglis commented 5 months ago

exec_() is being deprecated in favor of exec().

Also use launch() helper method for Dialog subclasses.

Note: there is one case of exec_() left, however I've found that under PySide2 QApplication does not have an exec() method. Also, this use of exec_ does not seem to produce warnings.

Fixes #595

deeplow commented 4 months ago

Thank for the PR and for fixing something which we've been neglecting for quite a while (since we added PySide6 support).

Note: there is one case of exec_() left, however I've found that under PySide2 QApplication does not have an exec() method. Also, this use of exec_ does not seem to produce warnings.

This fact was bothering me, so I dug a little deeper. PySide2 supported python2, which means that there it could not use exec since it was a reserved keyword (at least this plausible explanation for PyQt5 makes sense here as well).

This also explains why exec_() is not found on the C++ Qt5 docs (the PySide2 equivalent). On top of that, on the PySide2 docs, even though exec_() for QApplication has a docs entry, while exec() does not, the examples (which come from translated C++, I guess) use exec.

In conclusion, since Python2 has now reached its end of life, I think we can assume that exec() will be the de-facto way forward and that QApplication.exec_() working on PySide6 was just some developer forgetting to deprecate it. However, monkeypatching it after the PySide2 import is a bit of a mypy can of worms, so I guess we'll just have to leave it as-is for now :disappointed:.

In any case, if QApplication.exec_() ever stops working, we notice it, right away, so it's not a big deal.

naglis commented 4 months ago

This fact was bothering me, so I dug a little deeper. PySide2 supported python2, which means that there it could not use exec since it was a reserved keyword (at least this plausible explanation for PyQt5 makes sense here as well).

This also explains why exec_() is not found on the C++ Qt5 docs (the PySide2 equivalent). On top of that, on the PySide2 docs, even though exec_() for QApplication has a docs entry, while exec() does not, the examples (which come from translated C++, I guess) use exec.

Thank you for the extra analysis @deeplow! I was a bit baffled by exec() being mentioned in the Qt5 docs but the method not existing, now it is more clear why that might be the case.

apyrgio commented 4 months ago

Thanks a lot for the PR @naglis. Same as in #794, since we don't need to rush this PR, I'll merge it after the 0.6.1 release.