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 non-translated formatted string in js #12215

Merged

Conversation

n-peugnet
Copy link
Contributor

The formatted string was evaluated before the gettext call, which prevented any translated message from being loaded. Use a plain old string instead and manually replace the text afterwards.

Fixes #12129

I tried to switch to the supposedly more correct Document.ngettext function by the way, but didn't manage to make it work properly for french plurals. So instead I decided to only fix the original issue. This also allows to reuse the message for languages that have already translated it.

2024-03-30-215856_673x383_scrot

The formated string was evaluated befor the gettext call, which
prevented any translated message from being loaded. Use a plain old string
instead and manually replace the text afterwards.

Fixes sphinx-doc#12129.
Copy link
Member

@chrisjsewell chrisjsewell left a comment

Choose a reason for hiding this comment

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

Looks great to me thanks! Will leave it open a little longer, in case any one else wants to look

@picnixz
Copy link
Member

picnixz commented Apr 1, 2024

This is interesting. Are there other places where JS strings are not correctly localized ? (if so, could you correct them in one PR?)

@n-peugnet
Copy link
Contributor Author

This is interesting. Are there other places where JS strings are not correctly localized ? (if so, could you correct them in one PR?)

I didn't find other places with this problem. But I saw in the generated js translation files that there are some strings with variable replacements using a différent pattern, for example in the french translation's one:

"%(filename)s — %(docstitle)s": "%(filename)s — %(docstitle)s",
"© %(copyright_prefix)s %(copyright)s.": "",

Maybe it would be better to also use the same %(varname)s pattern in manually replaced strings?

For now I thought it would be best to keep the string as is, so we don't have to translate it again.

@picnixz
Copy link
Member

picnixz commented Apr 1, 2024

Maybe it would be better to also use the same %(varname)s pattern in manually replaced strings?

I'm sorry but I don't see the difference with what you showed (unless you are talking about the copyright line, in which case it appears we just don't translate it at all for now?)

For now I thought it would be best to keep the string as is, so we don't have to translate it again.

Yes, let's just not change it for now. I was just wondering whether there were other issues similar to what we had.

@picnixz picnixz merged commit 600d3d3 into sphinx-doc:master Apr 4, 2024
23 checks passed
@picnixz
Copy link
Member

picnixz commented Apr 4, 2024

Thank you!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Strings not localized
3 participants