XOOPS / XoopsCore

Core Framework for next version of XOOPS CMS: 2.6.0
https://xoops.org
138 stars 81 forks source link

Allow usage of placeholders and other goodies on translations #536

Closed trabisdementia closed 6 years ago

trabisdementia commented 7 years ago

This is a proposal for translations formatting.

Better way of explaining it is trough examples that you can find here: http://www.yiiframework.com/doc-2.0/guide-tutorial-i18n.html#message-formatting

I also added examples in the codex module.

It is 100% BC. Do not expect this code to break anything :)

Let us discuss it.

geekwright commented 7 years ago

Very interesting. Not sure I agree 100%, but see some benefits. It will take a little longer to fully digest this.

geekwright commented 7 years ago

Very nice in many ways, but I have reservations.

The dependence on Intl and ICU libraries is problematic. The availability and age of the libraries are constant issues. As is, it appears that functionality would vary greatly based on those factors.

I would choose a smaller feature set that was always available over a large feature set where availability cannot be counted on.

That is why CLDR through Punic was added. It gives us both the benefit of CLDR and an element of control over the environment that is lacking in Intl/ICU.

On the plus side, the named placeholders are very nice, and should be easier to work with that sprintf and "%2$s" like constructs.

Verdict, let's go with it for now, although, as with most things, there may be some changes in the implementation as we move forward.

trabisdementia commented 7 years ago

I share your reservations. I was thinking about it a little more and I realized that what we really need is placeholders and a way to handle plurals. The rest can be done with punic and does not need to be handled (by translators) inside the translation/message.

So I propose the following: All locales should extend \Xoops\Locale\AbstractLocale or a class that already extends it.

In AbstractLocale class we could add a method called format() that would accept a message and an array and do a simple search replace for placeholders.

Modules would us it as:

t::format(t::MY_MESSAGE, ['someKey' => 'someValue']);

or

t::format('MY_MESSAGE', ['someKey' => 'someValue']);

For handling plurals we would use methods instead of constants:

t::I_AM_YEARS_OLD(['years' => 'someValue']);

Translators would use simple php logic to handle the return value

Usage in smarty could be as following:

{assign myvalues ['years' => 0]}
{translate key="I_AM_YEARS_OLD" dirname="codex" values=$myvalues}

If values are present, use key as method. If method not found, use format. Else, check if key is defined, if not, return key.

What do you think? Simpler to use and perhaps better for translators since they do not have to learn new Language (ICU).

trabisdementia commented 7 years ago

It turns out that this class can handle the modifiers: number, select and plural! This ones trow an fatal error if php_intl is not installed:

            case 'date':
            case 'time':
            case 'spellout':
            case 'ordinal':
            case 'duration':
            case 'choice':
            case 'selectordinal':
                throw new NotSupportedException("Message format '$type' is not supported. You have to install PHP intl extension to use this feature.");

We could remove support for intl so we never trow an exception. Thoughts?

Btw, I did some changes, please check them.

geekwright commented 7 years ago

I thought the fallback for MessageFormatter would work for our needs.

I'm a bit tapped for time tonight. I'll sketch out the pieces for translations (includes plurals) and locale and we can make sure we have everything covered. I'll try and get that out tomorrow so we can discuss it.