Reported by GunChleoc on Transifex.
Details
Details
- Reviewers
elexis - Commits
- rP22121: Do not generate duplicate biome strings for translation
Tested locally.
Diff Detail
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Comment Actions
Thanks, it's my mistake to not have noticed the redundant strings and not have checked transifex regularly (at some point there was too much to follow)
| binaries/data/mods/public/l10n/messages.json | ||
|---|---|---|
| 691 ↗ | (On Diff #6760) | Title must remain, no? |
Comment Actions
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/differential/652/display/redirect
Comment Actions
Not a problem. In fact, I was the one that should have been watching Transifex, but instead I abandoned the project without a word. I’ll try to make it up.
| binaries/data/mods/public/l10n/messages.json | ||
|---|---|---|
| 691 ↗ | (On Diff #6760) | It is not necessary, the JSON files contain this: "Description": {
"Title": "…"
"Description": "…"
}The current behavior of the message extractor, as soon as it sees the root "Description", is to extract the values of all of its keys. |
Comment Actions
It still ends up as the same string on transifex, right? So people didn't have to do twice the work, just the public-maps.pot file was unclean?
The testing of your patch could be done by running source/tools/i18n/updateTemplates.py and diffing the 2 pot files, but in this case I trust you as the department head.
Also I agree that splitting the map JSON extraction into two parts, so that it is hard to not notice that the biome JSON files can be different from the strings in map JSON files.
Can restructure the JSON file as proposed in the inline comment sooner or later if you wish.
| binaries/data/mods/public/l10n/messages.json | ||
|---|---|---|
| 691 ↗ | (On Diff #6760) | I see. But then it still were preferable to only mention the strings directly and not every string that happens to be in the parent? |
Comment Actions
In Transifex, the string is currently appearing twice (which is why it was reported), because the biome-specific extraction settings include a context key, while the other data extraction did not. These context are meant to allow the biome strings to be translated on their own even if there are other exact matches somewhere else in the code, which may make sense if a biome has a common word as title.
After the change, the strings without the context dissapear. The ones with the biome context remain, which means that, if those were already translated in Transifex, they will continue to be translated.
The strings appearing twice in Transifex is an annoyance, but a minor one at that. At least for experienced translators that do not translate online on Transifex, but download the PO files for local translation and can use translation memories to quickly fill duplicate translation units.
I do not mind whatsoever to wait for the current freeze to be over before we apply this change. It should not break anything, but better to be safe than sorry.
Can restructure the JSON file as proposed in the inline comment sooner or later if you wish.
Great. I'll hold this patch until then. Thanks!
| binaries/data/mods/public/l10n/messages.json | ||
|---|---|---|
| 691 ↗ | (On Diff #6760) | Yes, if you could rename one of the two Description so that we do not have two matching keys of different levels of the JSON would be great. I just don't feel confident enough making such a change myself without breaking something. If you want to do this, I guess changing the parent one would make the most sense, so I would hold this patch, and update it after your change so that it keeps both keywords, as in the current settings. |