New feature: Translation message placeholder support

[TotalWipeOut]

Q	A
Documentation	not yet
Bugfix	no
BC Break	no
New Feature	yes
RFC	yes
QA	no

Description

WORK IN PROGRESS

Upon needing message placeholders and seeing the 4 year old issue #7 about needing it, I decided to take a stab at adding message placeholder support into laminas-i18n, as I see it as key missing feature.

(I decided to use the term 'placeholders' instead of 'parameters' so that it is less ambiguous)

~As I really didn't want a BC break, I have hijacked the $textDomain parameter of the translate / translatePlural method to allow passing in placeholders where it seemed most logical going forward. There is a way to be able to provide both textDomain and placeholders if that's needed.~

Goals

• Allow placeholders
• Support multiple kinds of placeholder formats
• No BC breaks
• 100% test coverage

What I've done so far

• Added a placeholder plugin manager
• Added initial set of placeholder compilers
• Modified Translator\TranslatorServiceFactory to inject a placeholder from config
• Modified Translator\Translator class to add new methods that pass messages through a placeholder compiler before returning
• New view helpers to utilise new Translator methods
• Seems to work at a basic level

• Existing tests are all passing

  TODO

  • have not yet written any tests for the new classes
  • The placeholders need work

Before i go any further, I am seeking advice as to if this is the correct approach for adding this without a bc break

The code is not perfect, so any guidance on naming/architecture/design would be amazing. Once i have some feedback, i will make any required changes and get on with the tests

And, a thank you to everyone in the Laminas community for being so kind and generous with your time :heart:

[froschdesign]

@TotalWipeOut First: Thank you very much for your help!

---

Some comments:

I decided to use the term 'placeholders' instead of 'parameters' so that it is less ambiguous

A placeholder is in the message: "Hello {name}!" and the process of converting the message with a placeholder can be called formatting or replacement. To format or replace the placeholders, values/parameters are required. Therefore, the naming with name "placeholders" in your code is incorrect.

As I really didn't want a BC break, I have hijacked the $textDomain parameter of the translate / translatePlural method to allow passing in placeholders where it seemed most logical going forward.

Hijacking is not an option because:

• the interface defines the parameter with the type string https://github.com/laminas/laminas-i18n/blob/b3259acf50899468cbbd105539ed0d5a5175c4d8/src/Translator/TranslatorInterface.php#L14
• with your solution, a rewrite of the arguments is required for the next major version
• and any hidden or magical use is not desired and must therefore be avoided

• Support multiple kinds of placeholder formats

My first thought was, why allow multiple variants when one official is enough, but if we see this component in use with different frameworks or CMS, this would be a benefit. On the other hand, with the ICU MessageFormat the translation with plural can be done with one method, and we can kill translatePlural.

• No new view helpers

Why not? We can update the view helper configuration to use the new view helpers with parameter support:

https://github.com/laminas/laminas-i18n/blob/b3259acf50899468cbbd105539ed0d5a5175c4d8/src/ConfigProvider.php#L151-L158

And the new view helper(s) can be used like this:

<?= $this->translate($message, params: ['name' => 'World']) ?>

The old view helpers must be retained for reasons of backward compatibility.

[TotalWipeOut]

@froschdesign Thanks for explaining placeholders vs parameters. I will adjust the code naming.

The reason for multiple placeholder formats is that while I agree ICU message format is the place to be, i wanted the ability to allow this feature to integrate with existing apps with existing translation files that might do it in the various other ways via custom solutions. The app I work on uses printf format for this task currently

Should I add a new method in Translator/Translator called translateWithParams or similar instead?

Regarding view helpers, from your example, can you show me how this will work with a new view helper, but called the same in the view and it not being a BC break? I don't fully understand

Thanks again for your detailed response :heart:

[froschdesign]

Regarding view helpers, from your example, can you show me how this will work with a new view helper, but called the same in the view and it not being a BC break? I don't fully understand

This is a simple mapping:

$renderer = new Laminas\View\Renderer\PhpRenderer();
$renderer->getHelperPluginManager()->configure(
    (new Laminas\I18n\ConfigProvider())->getViewHelperConfig()
);
$renderer->getHelperPluginManager()->setFactory(
    NewTranslate::class,
    Laminas\ServiceManager\Factory\InvokableFactory::class
);
$renderer->getHelperPluginManager()->setAlias(
    'translate',
    NewTranslate::class
);

echo $renderer->translate('example', params: ['foo' => 'bar']);

This overwrites the existing view helper, as is done if you want to use your own.

[TotalWipeOut]

@froschdesign So i understand correctly with this approach, this functionality requires additional config in the app, add wont work out of the box? If so, for something accessible out of the box, would adding a new 4th param to the Translate view helper be considered a bc break? As a view helper called translateWithParams feels too long for something that is used everywhere I see other frameworks use t or trans to keep it short, could we use a name like that? Thanks again for your time.

[froschdesign]

So i understand correctly with this approach, this functionality requires additional config in the app, add wont work out of the box?

This component already provides a configuration:

https://github.com/laminas/laminas-i18n/blob/b3259acf50899468cbbd105539ed0d5a5175c4d8/src/ConfigProvider.php#L151-L198

This must be extended so that the new view helper(s) can be used.

If so, for something accessible out of the box, would adding a new 4th param to the Translate view helper be considered a bc break?

No, because a new view helper must be created and the old one remains as it is. (The view helpers are not marked as final, so we cannot change them.)

As a view helper called translateWithParams feels too long for something that is used everywhere I see other frameworks use t or trans to keep it short, could we use a name like that?

No, nothing must and will be changed in this case. Via the configuration we will map translate to the new view helper which supports parameters. It is not necessary to change anything in the view scripts, unless you want to use the parameters, in which case the method call must be extended:

// Before:
$this->translate($message);

// After:
$this->translate($message, params: ['name' => 'World']);

This works out of the box.

---

Please look at the renderer of laminas-view:

• https://docs.laminas.dev/laminas-view/v2/php-renderer/#plugin
• https://docs.laminas.dev/laminas-view/v2/helpers/intro/

[TotalWipeOut]

@froschdesign Thanks - I have tried to address your feedback :)

[TotalWipeOut]

@froschdesign Let me know if this is something your happy to be included in this library, and i will get the tests written. Thanks CC @weierophinney for visibility, as you raised the original issue.

[froschdesign]

@TotalWipeOut Sorry for the late response! I was busy with this task: #90

Let me know if this is something your happy to be included in this library…

Short: The feature is definitely necessary, but with a modified implementation.

Minor Release

If we add this feature in a minor release then without any changes on the Translator class.

Example

namespace Laminas\I18n\Translator\Formatter;

use Laminas\I18n\Translator\Translator;
use Laminas\I18n\Translator\TranslatorInterface;

final class TranslatorFormatterDecorator implements TranslatorInterface
{
    public function __construct(
        private readonly Translator $translator,
        private readonly FormatterInterface $formatter
    ) {
    }

    public function translate(
        $message,
        $textDomain = 'default',
        $locale = null,
        iterable $parameters = []
    ): string {
        // …
    }

    public function translatePlural(
        $singular,
        $plural,
        $number,
        $textDomain = 'default',
        $locale = null,
        iterable $parameters = []
    ): string {
        // …
    }
}

Benefits

• no need to modify or extend the Translator class
• no need introduce an interface which is obsolete with the next major version
• the decorator:
  • can be added via delegator to the new view helpers
  • can easily removed with the next major version when the interface for the translator is modified
  • the user does not have to manually add or remove them

More Informations

• https://www.php.net/manual/en/language.oop5.basic.php#language.oop.lsp
• https://docs.laminas.dev/laminas-servicemanager/v4/delegators/

Major Release

Here we can change the interface for the translator and rework the entire Translator class.

---

So if you can wait, the next major version is an option.

[froschdesign]

@TotalWipeOut

CC @weierophinney for visibility, as you raised the original issue.

Not entirely true: "Originally posted by @thexpand at zendframework/zend-i18n#104" (See: #7)

[TotalWipeOut]

@TotalWipeOut

CC @weierophinney for visibility, as you raised the original issue.

Not entirely true: "Originally posted by @thexpand at zendframework/zend-i18n#104" (See: #7)

Oh yes, thanks for spotting that.