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 loading custom template translations for en #12220
base: master
Are you sure you want to change the base?
Fix loading custom template translations for en #12220
Conversation
The English language was hardcoded to load empty translations for templates as part of sphinx-doc#10067, but the rest of the code is already capable of handling missing translations just fine, so we can simply drop the first if block. This allows to use translations for English in custom templates, as for all the other languages. Fixes sphinx-doc#12214
Should I understand that, currently, we just assumed that |
What I understand is that before #10067, the translator was initiated with an empty set when the language was set to |
I see. What I would like to avoid however is to load all the catalogs (which could be expensive) if there is no reason to do that. Is there a way to known whether template localization or not? if so, it would be good to check whether the translator needs to be initialized or not. |
I'm not sure what you mean about "all the catalogs", this would only load le catalogs of the current language, if there are .po files for it. This change only remove an exception for the English language, and treats it as all the other ones. Here is an example build in a test project without any .po files for English:
We can see this message:
While I agree that this message could now be changed or removed, we can see that no translation catalogs have been loaded. And if a .po file exists for English, then it is more likely there for being used than for nothing. |
Ah, I'm sorry by saying "all catalogs". Actually, since I never used localization, I'm just wondering this could make the build much slower or not (maybe I should just have said that I was worried about the complexity overhead). If the build time does not increase, I think this PR is fine (but I would like someone else to review it since I'm not well-versed in this topic). |
Indeed, English is the source language of the Sphinx project, so there is no catalogs for it. Wee still need to keep the catalog loading for English, in case some translated strings are defined in custom templates in project which source language is not English.
I just added a commit that always show "done" for English language, as the "not available for built-in messages" message could be misleading. Now the only diff in the log is the addition of the message "loading translations [en]... done": --- before 2024-04-04 19:37:35.143948516 +0200
+++ after 2024-04-04 19:37:44.851948130 +0200
@@ -1,4 +1,5 @@
-Running Sphinx v7.2.6
+Running Sphinx v7.3.0+/bc127a0bf
+loading translations [en]... done
making output directory... done
building [mo]: targets for 0 po files that are out of date
writing output... This should not change anything for most of the users, Sphinx will simply look for English translations which most of the time will not be existing. So it will load an empty translation catalog. P.S. This message is now only printed for languages that do indeed not have any translations available for built-in messages: $ make html O=-Dlanguage=wtf
Running Sphinx v7.3.0+/bc127a0bf
loading translations [wtf]... not available for built-in messages |
If you want to be even more precise, you could also check whether the catalog is empty or not (is it even possible?) and say that the catalog is empty. But this would be just nitpicking and I think people just don't care about that. |
I'll let @chrisjsewell have a final look since I've never dealt with translations in general but it seems fine to me. |
…anslations-templates
I just fixed the merge conflict. |
The English language was hardcoded to load empty translations for templates as part of #10067, but the rest of the code is already capable of handling missing translations just fine, so we can simply drop the first if block.
This allows to use translations for English in custom templates, as for all the other languages.
Fixes #12214
(the diff is better looked at when hiding whitespace changes)