Bug in pylupdate breaks translations of some Black formatted calls

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
Post Reply
User avatar
chennes
Veteran
Posts: 3884
Joined: Fri Dec 23, 2016 3:38 pm
Location: Norman, OK, USA
Contact:

Bug in pylupdate breaks translations of some Black formatted calls

Post by chennes »

Yesterday I discovered a bug in pylupdate, the tool that extracts translation strings from our Python code for transmission to CrowdIn. The bug primarily affects code that has used the Black code reformatter.

The following is the output from Black for a line in the Addon Manager, and is legal, valid Python:

Code: Select all

translate(
    "AddonsInstaller",
    "Unable to read data from GitHub: check your internet connection and proxy settings and try again.",
)
Note the trailing comma that Black adds before the closing parenthesis: when Black splits up a line like this, it always adds the trailing comma. Unfortunately, the code in pylupdate does not support this syntax, and simply discards the entire translation string. I have submitted a bug to the Qt team here. I have a working fix here.

Note that I've only corrected it for translate(), not for tr(), which presumably suffers from the same issue. I also only corrected it for cases where there are two arguments, and no comment string. The bug also exists when there is a comment string but no pluralization info, and for when all four arguments are present along with a trailing comma. I don't see any instances of that in our codebase, so I have not bothered to address it. If someone at Qt decides to fix the bug hopefully they will handle all those cases.
Chris Hennes
Pioneer Library System
GitHub profile, LinkedIn profile, chrishennes.com
wmayer
Founder
Posts: 20243
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Bug in pylupdate breaks translations of some Black formatted calls

Post by wmayer »

If someone at Qt decides to fix the bug hopefully they will handle all those cases.
In the reply to your bug report they say the official tool to also handle Python files is now Qt's lupdate tool. Since you have a very recent Qt version you should additionally check with lupdate if the bug exists there, too. If not then I fear they won't fix anything.
User avatar
chennes
Veteran
Posts: 3884
Joined: Fri Dec 23, 2016 3:38 pm
Location: Norman, OK, USA
Contact:

Re: Bug in pylupdate breaks translations of some Black formatted calls

Post by chennes »

Yeah, it sounds like they didn't have a maintainer for pylupdate so dropped in in 6.x, in favor of everyone just using the C++ version. I think the tool mentioned in that reply, "pyside6-lupdate", is really just another name for "lupdate". Which is going to stop supporting QMake project files eventually (since Qt itself is moving to cMake, I guess).

What all this means is that a) for Qt 5.x we need our own version of pylupdate, since there clearly won't be any backports. And b) we need to redevelop our translation extractor tools for Qt 6.x to take this all into account. Once that's done I can test to see if lupdate extracts the strings correctly, which I strongly doubt, and then develop a fix for it, and then submit a bug report against the new code. It seems this tool is not used by many Python projects, as far as I can tell this bug has existed basically the entire time. Maybe while fixing it we can add support for f-strings :) .
Chris Hennes
Pioneer Library System
GitHub profile, LinkedIn profile, chrishennes.com
User avatar
chennes
Veteran
Posts: 3884
Joined: Fri Dec 23, 2016 3:38 pm
Location: Norman, OK, USA
Contact:

Re: Bug in pylupdate breaks translations of some Black formatted calls

Post by chennes »

OK, I can see that in Qt 6.2 the lupdate tool now directly supports Python, so that's a nice thing. And they have refactored it some, for the better I think: and that's also evidence that someone cares enough to work on it. Which is good :D . It does suffer from the same bug, but in the new version I believe it is even easier to fix. So I'll have a go at submitting another bug report...

(The code is here, if anyone else wants to look: https://code.qt.io/cgit/qt/qttools.git/ ... n.cpp#n519)

ETA: New bug report, now in the correct place (I hope!!) -- https://bugreports.qt.io/browse/QTBUG-100310
Chris Hennes
Pioneer Library System
GitHub profile, LinkedIn profile, chrishennes.com
User avatar
chennes
Veteran
Posts: 3884
Joined: Fri Dec 23, 2016 3:38 pm
Location: Norman, OK, USA
Contact:

Re: Bug in pylupdate breaks translations of some Black formatted calls

Post by chennes »

Just an update on this -- my patch was accepted, and the fix will appear in the Qt 6.4 release.

Here's the Bug:
QTBUG-100310

And the code review:
https://codereview.qt-project.org/c/qt/qttools/+/394245
Chris Hennes
Pioneer Library System
GitHub profile, LinkedIn profile, chrishennes.com
User avatar
Kunda1
Veteran
Posts: 13434
Joined: Thu Jan 05, 2017 9:03 pm

Re: Bug in pylupdate breaks translations of some Black formatted calls

Post by Kunda1 »

chennes wrote: Fri Feb 11, 2022 6:15 pm Just an update on this -- my patch was accepted, and the fix will appear in the Qt 6.4 release.
NICE!
Alone you go faster. Together we go farther
Please mark thread [Solved]
Want to contribute back to FC? Checkout:
'good first issues' | Open TODOs and FIXMEs | How to Help FreeCAD | How to report Bugs
Syres
Veteran
Posts: 2893
Joined: Thu Aug 09, 2018 11:14 am

Re: Bug in pylupdate breaks translations of some Black formatted calls

Post by Syres »

I'm pleasantly surprised by the quick and positive feedback, good work.
Post Reply