search

mottosso / Qt.py

Minimal Python 2 & 3 shim around all Qt bindings - PySide, PySide2, PyQt4 and PyQt5.
MIT License
896 stars 252 forks source link

fix translate arguments #379

Closed jdorr closed 1 year ago

jdorr commented 1 year ago

In this pull request, I update the logic in translate() to allow optional arguments.

I decided to open this pull request after I ran into a ui file compilation difference:

pyside2-uic foo.ui --> QCoreApplication.translate("MainWindow", u"MainWindow", None) (pyside-5.15.4)
pyside2-uic foo.ui --> QCoreApplication.translate("MainWindow", u"MainWindow", None, -1) (pyside-5.12.6)

Notice how the first compilation with pyside-5.15.4 returns translate with 3 arguments instead of 4. When we eventually use QtCompat.translate() we run into the following error:

  File "/dd/shows/DEV01/user/work.akelada/tools/cent7_64/package/launchpad/0.1.2/python/launchpad/Ui_launchpad_UI.py", line 205, in retranslateUi
    MainWindow.setWindowTitle(QtCompat.translate("MainWindow", u"MainWindow", None))
  File "/dd/tools/cent7_64/package/qt_py/1.3.6/python/Qt.py", line 844, in _translate
    "Expected 4 or 5 arguments, got {0}.".format(len(args) + 2))
TypeError: Expected 4 or 5 arguments, got 3.

Now translate("MainWindow", u"MainWindow", None) will work since n defaults to -1.

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

mottosso commented 1 year ago

Looks great to me! Only missing bit is the legal side - the CLA. That document there says that the code belongs to the repository rather than you the individual. It affects anyone downloading and using Qt.py and helps keep the legal monkeys out of the machine. Are you able to click through that?

jdorr commented 1 year ago

Strange, I signed the CLA 3x already but the status is still pending.

mottosso commented 1 year ago

Eek, unclear why that is. Would it be possible to make a new pull request, and try signing there instead? Could be the CLA service that's freaking out. If possible, can you try pasting the entire Qt.py file in the GitHub text editor of your fork, and committing from there? It should avoid any author confusion by both GitHub and CLA Assistant. Sorry for the hassle, lawyers man..

jdorr commented 1 year ago

Sounds good! I will try...

jandorr commented 1 year ago

Hi @mottosso, I was able to sign it in a new PR. Sorry for the confusion. https://github.com/mottosso/Qt.py/pull/382