Add tests for newlines in gettext catalogs #20815
Conversation
mzazrivec
force-pushed
the
add_tests_for_newlines_in_gettext_catalogues
branch
4 times, most recently
from
ded4e64
to
37af885
Compare
mzazrivec
force-pushed
the
add_tests_for_newlines_in_gettext_catalogues
branch
from
37af885
to
13cbfc9
Compare
mzazrivec
changed the title
mzazrivec
force-pushed
the
add_tests_for_newlines_in_gettext_catalogues
branch
from
13cbfc9
to
6eb704d
Compare
|
@miq-bot remove_label unmergeable |
|
@himdel review please? |
| @@ -47739,7 +47739,7 @@ msgstr "Esta funcionalidad no se admite en la versión API del proveedor" | |||
| #: | |||
| #: ../app/models/manageiq/providers/redhat/infra_manager/vm_or_template_shared/scanning.rb:51 | |||
| msgid "VMs must be scanned from an EVM server whose host is attached to the same\n storage as the VM unless overridden via SmartProxy affinity.\n Please verify that:\n 1) Direct LUNs are attached to ManageIQ appliance\n 2) Management Relationship is set for the ManageIQ appliance" | |||
| msgstr "Se deben analizar las MV desde un servidor EVM cuyo host esté conectado al mismo almacenamiento que la MV, a menos que se anule mediante afinidad con SmartProxy.\n Compruebe que:\n 1) Los LUN directos estén conectados a un dispositivo ManageIQ\n 2) La relación de gestión esté configurada para el dispositivo ManageIQ" | |||
| msgstr "Se deben analizar las MV desde un servidor EVM cuyo host esté conectado al mismo\n almacenamiento que la MV, a menos que se anule mediante afinidad con SmartProxy.\n Compruebe que:\n 1) Los LUN directos estén conectados a un dispositivo ManageIQ\n 2) La relación de gestión esté configurada para el dispositivo ManageIQ" | |||
There was a problem hiding this comment.
Random newline in the middle of a sentence? (we should probably only keep newlines after : or between sentences, right?
EDIT: never mind, this is already the case in the original msgid (newline in the middle of a sentence is rather suprrising, but it's not related to this PR)
spec/shared/i18n/newlines.rb
Outdated
| end | ||
|
|
||
| expect(boundary_errors).to be_empty | ||
| # We intentionaly do not expect overal_errors to be empty, since in certain cases it may be desirable |
There was a problem hiding this comment.
would it make sense to parametrize this?
If the tests don't go red on overall_errors, would we notice them? (Maybe we should skip it just in specific repos or for specific messages?)
EDIT: the issue is really that we have some newlines inside english text, that absolutely don't make sense to preserve in (shorter) japanese translations. So any parametrization would have to be per-language.
There was a problem hiding this comment.
Typo overal_errors -> overall_errors
There was a problem hiding this comment.
@Fryguy fixed, thanks!
|
@chessbyte Please review. |
mzazrivec
force-pushed
the
add_tests_for_newlines_in_gettext_catalogues
branch
from
6eb704d
to
c88288e
Compare
|
Some comments on commits mzazrivec/manageiq@f480af2~...c88288e spec/shared/i18n/newlines.rb
|
|
Checked commits mzazrivec/manageiq@f480af2~...c88288e with ruby 2.6.3, rubocop 0.82.0, haml-lint 0.35.0, and yamllint |
|
@chessbyte review / merge please? Thanks. |
Technically, we could try to fix this and get the translators to update their data to have this fixed, but it's possible there are situations where this happens again and it's not an actual problem for gettext. Even the original PR that added this said this overall number of newlines check could flag intentionally changed values. Since we've never reacted to this output, we can remove it. From that PR (we care about the 3rd check below): 1. if original string starts with a newline, translation needs to start with a newline too 2. if original string ends with a newline, translated string also needs to end with a newline 3. overal amount of newlines in original and translated string needs to match In this PR, I am introducing a new test, which tests the above rules. The exception is the 3rd rule, since it seems that in some places the newline might have been added (or omitted) intentionaly. So we're just logging these. Reference:: ManageIQ#20815
It seems that with lately added translation we have a new problem in our gettext catalogs: the translations do not honor newlines at string starts and string ends.
For example, from
locale/es/manageiq.po:Same thing shown in UI:
The rules that we need to enforce here:
In this PR, I am introducing a new test, which tests the above rules. The exception is the 3rd rule, since it seems that in some places the newline might have been added (or omitted) intentionaly. So we're just logging these.
This PR also fixes strings in catalogs that break the above rules.
@miq-bot add_label internationalization, test, wip