Translation linter work - Rework of #3597#3683
Conversation
Make script executable Add PR commenting logic Add pygithub dependency Fix import Remove GitHub requirements Refactor test suite Fix some errors Add styling Add severity Be closer to qtlinguist semantics
|
Please rebase this too such that we have no conflicts |
| def approximate_message_lines(text: str): | ||
| """Yield approximate line numbers for <message> elements.""" | ||
| lines = text.splitlines() | ||
| cursor = 0 | ||
| for _ in range(text.count("<message")): | ||
| for i in range(cursor, len(lines)): | ||
| if "<message" in lines[i]: | ||
| cursor = i + 1 | ||
| yield i + 1 | ||
| break | ||
| else: | ||
| yield 0 |
There was a problem hiding this comment.
Maybe add a TODO to think about how to not need to approximate the line numbers. I know it's from the original version and ChatGPT came up with it as hack.
There was a problem hiding this comment.
I tired something different. See if you think it's an improvement or I can revert.
ann0see
left a comment
ann0see
left a comment
There was a problem hiding this comment.
I'll need to check it myself locally again, but we should be ready soon.
Maybe. Not sure. Since it'll fail if there's an issue, the translation CI will complain. |
|
@ignotus666 maybe you could also have a look at the general concept of this script. It should find some issues (definitely not everything). |
|
Related: #2902 |
JaminShanti
force-pushed
the
translation-linter-work
branch
from
cc3fb03 to
4027304
Compare
|
Weblate already does all of these checks. I personally don't see the need for this - it's going to be constantly causing CI failures. |
|
Really? Most of those are based on what I'd check manually on PRs. So we may need to add other things. |
|
Weblate flags all those issues plus a few others (inconsistencies, markdown links, references and syntax, multiple capitals...). It just doesn't block anything on that basis, only providing warnings - it's up to translators to address the flagged issues. If they don't bother on Weblate I don't see how flagging them again here is going to change anything. It's often the case that they are false positives and can be ignored (you can tell Weblate to do that) due to particularities of different languages, or in the case of empty translations because that particular segment doesn't need translating. If the script here causes the CI build to fail I can guarantee you'll never see a successful build again. |
|
That's why it has severity levels. And we should absolutely fail if a translator does obvious mistakes and fix them. |
|
Fair enough, but from what I see it flags empty translations as "severe" and all the rest as "warning". If a segment is empty, won't it trigger the rest of the warnings as well? There are empty translations all over the place, because of incomplete translations and also because some segments don't need translating. An empty translation unit should never block the build. What I'm getting at is that it might be better practice to go to Weblate to look at flagged issues when there's a PR, instead of trawling through translations here and adding another layer of checks that are already performed there. |
|
Empty translation without unfinished must be severe. Empty translation with unfinished is ok as it falls back to the source. |
|
Anything that shouldn't need translation shouldn't be in the translations, really. We should break on build if they creep in - but, yes, there are a lot to fix. |
|
I was rather referring to things like, say, "Ok", or commands. The advice in the docs is that if the translation is the same as the source, to copy the source into the translation. But translators will often ignore that and leave it empty. |
|
So they need fixing one way or another to be compliant and avoid check errors. No point checking if no one is expected to fix things. Having the build fail does at least make them need fixing. |
|
Let me know if you want any changes. |
4 participants
Short description of changes
Rework of #3597
Introduces an extended Python-based Qt
.tstranslation checker. This tool validates translation files according to Qt Linguist semantics, catching common formatting and syntax issues such as:%1,%2, etc.)It features a strict mode (
--strict) designed for CI pipeline integration, which exits with a non-zero code when warnings or severe errors are present. It also outputs a detailed test summary organized by language.CHANGELOG: SKIP
Context: Fixes an issue?
Context: This provides a more robust, automated way to ensure the quality and consistency of internationalization (i18n) files, reducing the manual review overhead required for ongoing translation updates.
Does this change need documentation? What needs to be documented and how?
No end-user documentation is required on the website. A brief note should likely be added to
CONTRIBUTING.mdor the developer documentation explaining how contributors or CI runners can execute the script against thesrc/translationdirectory.Status of this Pull Request
Working implementation.
What is missing until this pull request can be merged?
Checklist