-
Notifications
You must be signed in to change notification settings - Fork 27
Complete language support to all GUI tabs #873
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
Open
Ski90Moo
wants to merge
18
commits into
openstudiocoalition:develop
Choose a base branch
from
Ski90Moo:develop
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
2180c36
i18n: wrap ~76 user-facing strings with tr() across Schedules tab
Ski90Moo aff35f2
ci: trigger build
Ski90Moo b1c5733
i18n: add comprehensive Spanish translation proof-of-concept (issue #…
Ski90Moo bce5a70
test: add GTest suite for i18n translation (issue #680)
Ski90Moo cdf9504
i18n: add full translations for 18 languages + 4 new languages (issue…
Ski90Moo d239658
i18n: fix remaining macumber/Copilot review issues (issue #680)
Ski90Moo 7eb3da0
chore: untrack Qt-generated moc_*.cpp files
Ski90Moo f03e2ee
chore: ignore Qt-generated moc_*.cpp files
Ski90Moo 4808ad1
test: add IddCoverageForAllFields test (issue #680)
Ski90Moo 59cad72
test: check IDD coverage across all language .ts files (issue #680)
Ski90Moo b2a2d08
docs: update PR 873 review responses
Ski90Moo 7e3be47
ci: add GitHub Actions translation drift check (issue #680)
Ski90Moo 518b74f
docs: update PR 873 responses with translation CI details
Ski90Moo d74d92b
i18n: wrap hardcoded 'Merging Models' dialog titles with tr() (issue …
Ski90Moo 05a157e
refactor: replace QCoreApplication::translate lambda/verbose calls wi…
Ski90Moo 8d3ebce
fix: add missing QStringList include to Translation_GTest.cpp
Ski90Moo dce7d74
chore: remove internal PR review tracking document
Ski90Moo 37c8d6b
i18n: add dialog translations missing from PR #873
Ski90Moo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| name: Translation Check | ||
|
|
||
| on: | ||
| push: | ||
| branches: [ master, develop ] | ||
| pull_request: | ||
| branches: [ master, develop ] | ||
|
|
||
| jobs: | ||
| check: | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Install Qt6 lupdate | ||
| run: | | ||
| sudo apt-get update -q | ||
| sudo apt-get install -y qt6-l10n-tools | ||
|
|
||
| - name: Run lupdate against all .ts files | ||
| run: | | ||
| # -locations none: suppress line-number comments so only genuine | ||
| # string additions/removals cause a git diff change. | ||
| lupdate6 src/ \ | ||
| -ts translations/OpenStudioApp_*.ts \ | ||
| -extensions cpp,hpp \ | ||
| -locations none \ | ||
| 2>&1 | tee lupdate_output.txt | ||
|
|
||
| - name: Detect vanished or new unfinished strings | ||
| run: | | ||
| # If lupdate changed any .ts file, strings were either added to source | ||
| # (new type="unfinished" stubs) or removed from source (type="obsolete"). | ||
| # Either way the commit that introduced the drift should not merge until | ||
| # the .ts files are updated and translations are provided. | ||
| python3 ci/check_translations.py | ||
|
|
||
| - name: Upload artifacts on failure | ||
| if: failure() | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: translation-check-${{ github.sha }} | ||
| path: | | ||
| lupdate_output.txt | ||
| translation_check.patch |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| #!/usr/bin/env python3 | ||
| """ | ||
| Called by translation_check.yml after lupdate has run. | ||
|
|
||
| Checks whether lupdate changed any OpenStudioApp_*.ts file by running | ||
| `git diff translations/`. If there are changes it means: | ||
|
|
||
| - type="unfinished" stubs were ADDED → new tr() calls in source with no | ||
| translation yet. Fix: run translate_skeleton.py / translate_all_languages.py. | ||
|
|
||
| - type="obsolete" entries were ADDED → tr() calls removed from source but | ||
| still present as dead entries in the .ts files. Fix: run | ||
| `lupdate -no-obsolete` and commit the cleaned-up .ts files. | ||
|
|
||
| If the diff is empty, all .ts files are in sync with the C++ source. | ||
| Exits with code 1 if drift is detected so CI fails. | ||
| """ | ||
|
|
||
| import re | ||
| import subprocess | ||
| import sys | ||
|
|
||
|
|
||
| def run(cmd: list[str]) -> str: | ||
| return subprocess.check_output(cmd, text=True) | ||
|
|
||
|
|
||
| def main() -> int: | ||
| # Produce a diff of everything lupdate may have touched. | ||
| diff = run(["git", "diff", "translations/"]) | ||
|
|
||
| if not diff: | ||
| print("All .ts files are in sync with the C++ source. ✓") | ||
| return 0 | ||
|
|
||
| # Save patch as artifact for inspection. | ||
| with open("translation_check.patch", "w") as f: | ||
| f.write(diff) | ||
|
|
||
| # Classify what changed so the error message is actionable. | ||
| added_unfinished = len(re.findall(r'^\+.*type="unfinished"', diff, re.MULTILINE)) | ||
| added_obsolete = len(re.findall(r'^\+.*type="obsolete"', diff, re.MULTILINE)) | ||
|
|
||
| # Summarise changed files. | ||
| stat = run(["git", "diff", "--stat", "translations/"]) | ||
| print("lupdate changed the following .ts files:") | ||
| print(stat) | ||
|
|
||
| if added_unfinished: | ||
| print(f" {added_unfinished} new empty unfinished string(s) detected.") | ||
| print(" These are new tr() calls in C++ source that have no translation yet.") | ||
| print(" Fix: run translate_skeleton.py (Spanish) or translate_all_languages.py") | ||
| print(" to batch-translate the new strings, then commit the updated .ts files.") | ||
| print() | ||
|
|
||
| if added_obsolete: | ||
| print(f" {added_obsolete} new obsolete string(s) detected.") | ||
| print(" These are tr() calls that were removed from C++ source.") | ||
| print(" Fix: run `lupdate src/ -no-obsolete -ts translations/OpenStudioApp_*.ts`") | ||
| print(" then commit the cleaned-up .ts files.") | ||
| print() | ||
|
|
||
| if not added_unfinished and not added_obsolete: | ||
| # Location comments or other minor changes — still fail so the dev is aware. | ||
| print(" .ts files changed in an unexpected way (not unfinished/obsolete).") | ||
| print(" Review translation_check.patch for details.") | ||
|
|
||
| print("Translation check FAILED. Commit the updated .ts files to fix CI.") | ||
| return 1 | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| sys.exit(main()) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really cool, I just wonder how we will be able to maintain this. The Translation test could possibly loop over each IddObject and verify that there is an entry for each one?
Same concern for the OutputVariables
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was one my concerns as well. I did a handful of them, but I could imagine the list of these objects would be enormous and difficult to maintain. It also makes the .ts file huge; no small task for the language contributors to review. I envision it would be more of a reactionary process whereby the contributors review the general GUI stuff, but we would get feedback as users stumble upon poorly translated objects.
The OutputVariables should be complete though. I will look into modifying the Translation test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call. The OutputVariable gap is fixed — the original entries only had
Zone Xprefixed names, butoutputVariableNames()returns bare names (e.g.Lights Electricity Energy). Added 359 bare-name entries auto-derived from the Zone translations, plus specificSite/Surface/People/Daylightingentries.For IDD fields: all fields now have translations across all 18 language
.tsfiles, generated via Claude API batch translation. Two complementary tools maintain coverage:add_idd_skeleton.py— developer maintenance tool. Run after an SDK update to add skeleton entries for any new IDD fields, then runtranslate_skeleton.pyto batch-translate them.Translation_ts.IddCoverageAllLanguagesGTest — usesIddFactory::instance()to enumerate every IddObject and its fields, then checks that each unique field name has a source entry in the IDD context of everyOpenStudioApp_*.tsfile. Fails with a per-language list of missing fields if any file is out of sync.The test detects drift; the script fixes it. They run independently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you forget to add add_idd_skeleton.py and translate_skeleton.py to the pr?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in commit
44b3e7d7. The two scripts are now in the repo root:add_idd_skeleton.py— scansOS_*_FieldEnums.hxxunder the OpenStudio build tree to find field names not yet in theIDDcontext and inserts empty<message>skeleton entries. IDD directory is auto-detected by glob or passed as a CLI argument.translate_skeleton.py— batch-translates those unfinished entries via the Anthropic Messages Batches API (readsANTHROPIC_API_KEYfrom the environment).The
IddCoverageAllLanguagestest added in this PR already provides the CI enforcement side: it loops over every IDD object's fields viaIddFactoryand fails if any field name is missing from theIDDcontext in a.tsfile.