-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[#4968] Fix multiline translations - Take 2 #5339
Conversation
backporting might break theme plugin translation overrides that expect the current whitespace, but maybe it's fine to just mention this in the release notes? |
+1 for backporting. Eventually, every portal will be upgraded and the maintainer will have no choice but to do something with custom overrides. So adding clear notice about translation update would be ok - there are not so many multiline strings, so, I hope, it won't take much effort to do and update inside your extension while applying patch releases. |
There are 12 strings that were broken because of this, but some of them are quite prominent like the Groups and Orgs helper text (which are the ones that people will have customized if they wanted to rename them). It is true that we can break some existing translation but I lean towards backporting and documenting because:
I'll add the notice in the changelog as part of this PR so you can check if it is informative enough |
@smotornyuk @wardi if any of you has some time can you review and merge if happy? Thanks! |
✔️ reviewed. Need to fix conflicts and then it's ready to merge |
Thanks @smotornyuk! |
This was not backported in CKAN 2.8.4. |
I decided not to backport this to 2.8 at this stage. The Jinja2 version that 2.8 uses ( |
Fixes #4968
This builds on #5338 but it goes beyond with what I think is a better approach.
For reference, this is a string extracted from a Jinja2 multiline block (a
msgid
) in the pot file in current master:As @smotornyuk correctly pointed in #5338, using the standard Jinja2 Babel extractor save us some custom code that we were using in our extractor. But as it is, #5338 extracts messages as:
If we use the above msgid in the po files, the string gets correctly translated. But by essentially keeping all existing whitespace including line breaks in the msgid, it makes them much more fragile as any whitespace change in the templates will result in a new msgid, and the existing translation will be lost.
I propose we use the
ext.i18n.trimmed
policy both when extracting messages (da39d7e) and when parsing translated blocks (4cf6e0a) to avoid any whitespace differences.This is how the string gets extracted with this policy:
Now, this gives us more robust msgids going forward, but as @smotornyuk correctly pointed out in #4968 if we update them now in the PO files, the current existing translations (
msgstr
) will be lost. To avoid this I wrote a small command that updates the msgids in the PO files with the corresponding new msgid in the POT file. You can see in the PO files in the PR that only the multiline strings were updated.If the suggested approach makes sense to all, I'd like to backport this to 2.8. Here's the intended process:
@smotornyuk @wardi Does this make sense?