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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Localization: allow for more text strings to be translated #2428

Merged

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Jul 8, 2021

In a number of places in the code base I came across hard-code error messages, which were not set up to be translatable.

For those I found, I've now fixed this.

馃憠馃徎 Note: I've not added the labels used for logging errors via the PHPMailer:edebug() method. If so desired, that could be added in a future iteration.

馃憠馃徎 Existing translation files have not been updated. Translators may need to be pinged before the next release to update the translation(s) they maintain.

[Edit]
Includes a minor tweak to the PHPCS configuration to allow lines in language files to exceed the "120 chars line max" as using concatenation for text strings in translation files will not work with the current way of loading these files and the new "buggy PHP" message is a long text which would result in nearly all translation files throwing the warning.

@Synchro
Copy link
Member

Synchro commented Jul 8, 2021

There are no translators to ping! All translations have been submitted randomly, and only very rarely have they ever been revised by the same person! This is why I've been hesitant to introduce new translations in the past (because adding one also invites 50 PRs!), and also in light of what I mentioned before about the translations not actually being that useful.

What would be more useful is what was requested in #1579: error codes for every possible problem. They would make translation mapping simpler too. However, it would also be hard to do without a chunky BC break. There is also the issue that we do need to change the format of translation files to something non-executable, like yaml, JSON, po; the current parsing mechanism introduced recently is very much a stop-gap.

I'm happy to merge this anyway as it does improve consistency, but I just though I'd provide some context.

@jrfnl
Copy link
Contributor Author

jrfnl commented Jul 8, 2021

@Synchro Thanks for the context. When I read through it though, those improvements would all still need a comprehensive "base" language string list and this PR just makes the existing list more complete and the code more consistent regarding localization, so could be seen as a preliminary step.

Re: 50 translation PRs - want me to reduce it by one ? I can adjust the Dutch file in a second commit in this PR if you like.

@Synchro
Copy link
Member

Synchro commented Jul 9, 2021

Sounds good to me!

jrfnl added 2 commits July 9, 2021 15:01
In a number of places in the code base I came across hard-code error messages, which were not set up to be translatable.

For those I found, I've now fixed this.

Note: I've not added the labels used for logging errors via the `PHPMailer:edebug()` method. If so desired, that could be added in a future iteration.

Existing translation files have not been updated. Translators may need to be pinged before the next release to update the translation(s) they maintain.

Includes a minor tweak to the PHPCS configuration to allow lines in language files to exceed the "120 chars line max" as using concatenation for text strings in translation files will not work with the current way of loading these files and the new "buggy PHP" message is a long text which would result in nearly all translation files throwing the warning.
@jrfnl jrfnl force-pushed the feature/localization-add-missing-text-strings branch from f9c3a7f to 683a503 Compare July 9, 2021 13:04
@jrfnl
Copy link
Contributor Author

jrfnl commented Jul 9, 2021

Update for the Dutch translation file added + I've added one small extra change to the original commit (PHPCS config tweak).

@Synchro Synchro merged commit 687521c into PHPMailer:master Jul 9, 2021
@Synchro
Copy link
Member

Synchro commented Jul 9, 2021

Thanks again! Good call on the phpcs tweak.

@jrfnl jrfnl deleted the feature/localization-add-missing-text-strings branch July 9, 2021 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants