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

Provide context field to translation strings #19120

Merged
merged 1 commit into from
Jun 12, 2023
Merged

Conversation

Chocobo1
Copy link
Member

@Chocobo1 Chocobo1 commented Jun 7, 2023

No description provided.

@Chocobo1 Chocobo1 added the Code cleanup Clean up the code while preserving the same outcome label Jun 7, 2023
@Chocobo1 Chocobo1 added this to the 4.6.0 milestone Jun 7, 2023
@Chocobo1 Chocobo1 requested a review from a team June 7, 2023 11:07
@Chocobo1 Chocobo1 changed the title Provide context to translation strings Provide context field to translation strings Jun 7, 2023
@glassez
Copy link
Member

glassez commented Jun 7, 2023

Note that many of translations affected by this patch are never actually used (since they are used before translator is initialized). So maybe drop these strings from being translated at all?

@Chocobo1
Copy link
Member Author

Chocobo1 commented Jun 7, 2023

Note that many of translations affected by this patch are never actually used (since they are used before translator is initialized). So maybe drop these strings from being translated at all?

If my reading is correct, cmdoptions.cpp and upgrade.cpp is affected. Or move the translator initialization earlier? (before upgrade.cpp)

@Chocobo1
Copy link
Member Author

Chocobo1 commented Jun 7, 2023

So maybe drop these strings from being translated at all?

Wouldn't this become a review/maintenance burden? Maybe in the future, you and contributors will forgot and still add tr() then?

@thalieht
Copy link
Contributor

thalieht commented Jun 7, 2023

Note that many of translations affected by this patch are never actually used (since they are used before translator is initialized). So maybe drop these strings from being translated at all?

If my reading is correct, cmdoptions.cpp and upgrade.cpp is affected.

Maybe i didn't understand what you guys are talking about but i tested before this patch qbittorrent --help and .e.g. "Display program version and exit" is translated, all of them actually (tested with Deutch).

@Chocobo1
Copy link
Member Author

Chocobo1 commented Jun 11, 2023

Maybe i didn't understand what you guys are talking about but i tested before this patch qbittorrent --help and .e.g. "Display program version and exit" is translated, all of them actually (tested with Deutch).

I verified it on Windows. It seems Qt defaults to OS locale settings and qbt overwrites it later. So dropping translation strings is not a good idea.

@glassez
Copy link
Member

glassez commented Jun 11, 2023

It seems Qt defaults to OS locale settings and qbt overwrites it later.

It is just processed after translation is initialized.
On closer inspection there are quite a few strings that are processed before translation is initialized.

@Chocobo1 Chocobo1 merged commit 81bc910 into qbittorrent:master Jun 12, 2023
@Chocobo1 Chocobo1 deleted the tr branch June 12, 2023 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code cleanup Clean up the code while preserving the same outcome
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants