Skip to content
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

Fix bad placeholders in translated strings #5852

Merged
merged 1 commit into from
Jul 4, 2024

Conversation

impaktor
Copy link
Member

@impaktor impaktor commented Jul 2, 2024

This mostly fixes #5849 (translators getting the placeholders in strings wrong), BUT:

  • I assume we don't want to merge this PR, but push branch to transifex
  • Ignoring lang/core/ for now, due to some parsing issue with % and it getting close to my bed time (I'll fix tomorrow), that will add < 10 more files with corrections
  • IF this PR is merged to master, I could include the script I wrote to detect and fix the errors.
  • As mentioned in the issue above, this only solves for strings where number of placeholders in translated and original strings are the same / unambiguous)

Please have a scroll of the diff. (Ping @zonkmachine, @JeromeChignoli

@impaktor impaktor force-pushed the fix-translated-strings branch 2 times, most recently from 0ae2669 to 410e4bd Compare July 3, 2024 10:01
@impaktor
Copy link
Member Author

impaktor commented Jul 3, 2024

I went through the diff and noticed many (of the few) strings that the automated script got wrong were of the type: one of the place holders misspelled, but placeholders used in different order. I've fixed those, and updated this PR

Remains to fix: files in core with %
EDIT: Updated, and ready to push to transifex.

@@ -1105,7 +1105,7 @@
},
"OVER_N_BILLION": {
"description": "",
"message": "Alrededor de %{population}000 millones"
"message": "Alrededor de %%population000 millones"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need to be manually fixed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I spotted that one, and I initially assumed the script was doing something wonky, but it is actually doing what it is supposed to:

2024-07-03 14:19:40.474 | DEBUG    | __main__:compare_file:127 - translated: OVER_N_BILLION, ['{population}']
2024-07-03 14:19:40.474 | DEBUG    | __main__:compare_file:128 - correct:    OVER_N_BILLION, ['%population']
2024-07-03 14:19:40.474 | DEBUG    | __main__:fix_single_typo:96 - guess: %population, wrong: {population}
2024-07-03 14:19:40.474 | DEBUG    | __main__:fix_single_typo:98 - suggested: ['%population']
2024-07-03 14:19:40.501 | INFO     | __main__:fix_file:215 - old: Alrededor de %{population}000 millones
2024-07-03 14:19:40.501 | INFO     | __main__:fix_file:217 - new: Alrededor de %%population000 millones

My plan is to just open an issue on the strings that are "beyond broken" in transifex, and hope translator fixes it.

@@ -1021,7 +1021,7 @@
},
"NUMBER_HOURS": {
"description": "",
"message": "%hours{f.2} óra"
"message": "%hours{f.1} óra"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is technically a valid change in format specifier.

This mostly fixes pioneerspacesim#5849 (translators getting the placeholders in strings
wrong), BUT:

- I assume we don't want to merge this PR, but push branch to transifex

- As mentioned in the issue above, this only solves for strings where number
  of placeholders in translated and original strings are the same / unambiguous)
@impaktor impaktor marked this pull request as ready for review July 4, 2024 13:14
@impaktor
Copy link
Member Author

impaktor commented Jul 4, 2024

I've pushed these strings to transifex now, and I believe this PR should also be merged, before next time the bot runs.

@impaktor impaktor merged commit d5e3147 into pioneerspacesim:master Jul 4, 2024
@impaktor impaktor deleted the fix-translated-strings branch July 4, 2024 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bad keys in translation files
2 participants