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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Translation parameters support #7

Open
weierophinney opened this issue Dec 31, 2019 · 15 comments
Open

Translation parameters support #7

weierophinney opened this issue Dec 31, 2019 · 15 comments

Comments

@weierophinney
Copy link
Member

The Idea

Recently we talked on Slack about having parameters support for translated strings.
I made a research of how other frameworks are already doing it.

Other Frameworks

Here is the documentation on how it is implemented in Symfony and in Laravel.

Symfony

I like how Symfony uses PHP's strtr here: https://github.com/symfony/translation/blob/master/Formatter/MessageFormatter.php#L42-L45
However, I don't like the usage with those percent % characters as they make the string less readable, e.g. You scored %scored_points% points and you pass the exam with %score%% success!.

Laravel

I don't like Laravel's replacement implementation as this is a fairly large amount of code for such a simple task. However, I like how they name their parameters with a leading colon :, eg. :scored_points, :score, which makes the string a little bit easier to read. The previous example would become You scored :scored_points points and you pass the exam with :score% success!

My suggested approach

It will be best to use colon-prefixed parameters (readability) and strtr, which takes into account other parameters' names, regarding of the order they are defined (in contrast to Laravel's approach to use str_replace with sorted parameters), meaning that if the parameterized string contains both :score and :scored_points, the value for former would not overwrite the latter, unless the parameter has not been passed.

Implementation

I want to do and will do a PR, but first I want to discuss with you the idea and what are the possible solutions, taking into account the current state of this component. I would like to hear your opinion and suggestions.

The current methods can be changed, thus introducing a BC Break (the feature will be released with the next major version). Currently there are 2 methods in the TranslatorInterface: translate and translatePlural, which may be modified as follows:

/**
 * Translate a message.
 *
 * @param  string $message
 * @param  array  $parameters
 * @param  string $textDomain
 * @param  string $locale
 * @return string
 */
public function translate(
    $message,
    array $parameters = [],
    $textDomain = 'default',
    $locale = null
);

/**
 * Translate a plural message.
 *
 * @param  string      $singular
 * @param  string      $plural
 * @param  int         $number
 * @param  array       $parameters
 * @param  string      $textDomain
 * @param  string|null $locale
 * @return string
 */
public function translatePlural(
    $singular,
    $plural,
    $number,
    array $parameters = [],
    $textDomain = 'default',
    $locale = null
);

Originally posted by @thexpand at zendframework/zend-i18n#104

@weierophinney
Copy link
Member Author

Placeholders with "%placeholder%" are already in use in the Zend-Form component. They have the huge benefit that you can preg_search your whole HTML body for unreplaced placeholders. If you forget the value for ":scored_points" the output will use the value of ":score" twice and scramble the output.

if (preg_match_all('~%[a-z0-9-_]+%~iu', $response, $matches) > 0) {
    // oh uh!
}

Also the length of the placeholder is clearly marked, so the order of replacement is irrelevant. (Well except in erroneous (?) cases where you replace one placeholder with another, the value of parameter "%scored_points%" is "%score%"...).

":placeholder:" is in use in the zend-router component, but they are for route-parameters. "%placeholder%" in the form component is directly used for placeholder in translations.


Originally posted by @MatthiasKuehneEllerhold at zendframework/zend-i18n#104 (comment)

@weierophinney
Copy link
Member Author

@MatthiasKuehneEllerhold Erroneous cases like the one you mentioned by replacing one placeholder with another is prevented with strtr, so that won't be an issue. Consider the following example:

<?php
$message = 'You scored %score% points with a %score_percentage%% success!';

// Outputs "You scored %score_percentage% points with a 99% success!"
echo strtr($message, [
    '%score%' => '%score_percentage%',
    '%score_percentage%' => 99,
]);

Originally posted by @thexpand at zendframework/zend-i18n#104 (comment)

@weierophinney
Copy link
Member Author

Sorry if I mixed my points.

You said for using str_replace we need to sort all placeholders by length. Which strtr conveniently does for us (probably faster than via userland sort).

I meant that the order of replacement is only relevant if we're using placeholders without end markers (e. g. ":placeholder"). If we're using end markers (e. g. "%placeholder%") the order is irrelevant except in the mentioned erroneous case.

For example using str_replace without sorting:

message = 'You scored :score points with a :score_percentage% success!';
echo str_replace(
  $message, 
 [
    ':score' => 50,
    ':score_percentage' => 99,
  ]
);
// Outputs "You scored 50 points with a 50_percentage% success!"

Using either strtr would've prevented the false replacement or a length-sort on the placeholders beforehand.

But using end-markers this can't happen:

message = 'You scored %score% points with a %score_percentage%% success!';
echo str_replace(
  $message, 
 [
    '%score%' => 50,
    '%score_percentage%' => 99,
  ]
);
// Outputs "You scored 50 points with a 99% success!"

TL;DR: I think the end-markers are direly necessary.


Originally posted by @MatthiasKuehneEllerhold at zendframework/zend-i18n#104 (comment)

@weierophinney
Copy link
Member Author

Yes, I understand you now. So basically we have 2 cases here.
Clearly, if the :score_percentage parameter is omitted, but :score is present, it will do a false replacement that will result in an unexpected behavior. So, it totally makes sense to use end-markers, too. We just have to decide what the markers should look like. I'm okay with %, I just thought initially that they make the string a little bit harder to read.

Having that sorted out, the next thing to decide is whether to use the str_replace or strtr. I did not do a benchmark myself, but from what I've read str_replace is faster for longer replacement strings. However, performance here might not be an issue. The problem I see with str_replace is the one you already pointed out previously - someone might set a parameter name as the value of another parameter. Using strtr sorts out this thing and we won't have to deal with sorting parameters by length.

Next thing on the list is whether the usage will be (1) or (2).
We can clearly see that Symfony uses (2), but I don't think it is good for one particular reason - it is not poka-yoke. Personally I prefer (1), because it is poka-yoke - there is one and only one way to do it. People can't get confused. The way they would define parameters in the message will be %param% and they will pass the name of the param in the array as param. End of story - no confusion.

// 1. No marks in the parameters
$this->translate('You scored %score% points', ['score' => 100]);

// 2. Marks in the parameters
$this->translate('You scored %score% points', ['%score%' => 100]);

Originally posted by @thexpand at zendframework/zend-i18n#104 (comment)

@weierophinney
Copy link
Member Author

We're using internally (1) (no marks in the parameters) because this way its easier for the end user to define the parameters. And thats why we're using foreach and str_replace.
Using a bulk replace via strtr or str_replace with (2) would be much faster I reckon.


Originally posted by @MatthiasKuehneEllerhold at zendframework/zend-i18n#104 (comment)

@FabianKoestring
Copy link

Originally posted in 21 Sep 2018 and still no pull request. @thexpand do you still want to work on this?

@thexpand
Copy link
Contributor

@FabianKoestring Yes, I've put it aside, because of the transition to Laminas and because that would be a BC break. Shall I start working on this?

@FabianKoestring
Copy link

Shall I start working on this?

Hey @thexpand it is up tou you. Would be nice 😜👍

@froschdesign
Copy link
Member

@thexpand
Please keep in mind that you should follow a format which is widely used and is not an isolated / proprietary solution. Which brings us to "ICU Message Format".

See also: "The Missing Guide to the ICU Message Format – Which i18n Libraries are Using the ICU Message Format?"

@ChibuezeAgwuDennis
Copy link

I hope I love the way Codeigniter 4 implemented text arguments Hello {name},. That is lang('Hello {name},', ['name' => 'Chibueze']). So for here it can be
$this->translate('You scored {score} points', ['score' => 100]);. Codeigniter uses the php MessageFormater

@froschdesign
Copy link
Member

@ChibuezeAgwuDennis

Codeigniter uses the php MessageFormater

Symfony also uses PHP's MessageFormatter: https://symfony.com/doc/current/reference/formats/message_format.html

Since this component is based on PHP's intl extension, the MessageFormatter class should also be used here.

@ChibuezeAgwuDennis
Copy link

@ChibuezeAgwuDennis

Codeigniter uses the php MessageFormater

Symfony also uses PHP's MessageFormatter: https://symfony.com/doc/current/reference/formats/message_format.html

Since this component is based on PHP's intl extension, the MessageFormatter class should also be used here.

I believe with this { instead of this % codeigniter team try avoid the percentage issue

@froschdesign
Copy link
Member

I believe with this { instead of this % codeigniter team try avoid the percentage issue

They follow the ICU MessageFormat syntax:

MessageFormat

The ICU MessageFormat class uses message "pattern" strings with variable-element placeholders (called “arguments” in the API docs) enclosed in {curly braces}.

https://unicode-org.github.io/icu/userguide/format_parse/messages/#messageformat

@thexpand
Copy link
Contributor

I see there is some movement here. Sadly, I no longer code in PHP unless it's part of a legacy project that needs migrating over to something else. I'm sorry that I didn't pick this up earlier but at the time I didn't have enough resource to spare to start working on it. I hope someone has the time to develop this, because otherwise it would've been a discussion gone to waste.

TotalWipeOut added a commit to TotalWipeOut/laminas-i18n that referenced this issue May 16, 2024
TotalWipeOut added a commit to TotalWipeOut/laminas-i18n that referenced this issue May 16, 2024
TotalWipeOut added a commit to TotalWipeOut/laminas-i18n that referenced this issue May 16, 2024
TotalWipeOut added a commit to TotalWipeOut/laminas-i18n that referenced this issue May 16, 2024
TotalWipeOut added a commit to TotalWipeOut/laminas-i18n that referenced this issue May 16, 2024
@TotalWipeOut
Copy link

I have taken a stab at adding this feature, please take a look at the implementation if you have time. Any feedback would be great.
#121

TotalWipeOut added a commit to TotalWipeOut/laminas-i18n that referenced this issue May 16, 2024
Signed-off-by: Kevin Hamilton <kevin@eagleeye.com>
TotalWipeOut added a commit to TotalWipeOut/laminas-i18n that referenced this issue May 16, 2024
Signed-off-by: Kevin Hamilton <kevin@eagleeye.com>
TotalWipeOut added a commit to TotalWipeOut/laminas-i18n that referenced this issue May 16, 2024
Signed-off-by: Kevin Hamilton <kevin@eagleeye.com>
TotalWipeOut added a commit to TotalWipeOut/laminas-i18n that referenced this issue May 16, 2024
Signed-off-by: Kevin Hamilton <kevin@eagleeye.com>
TotalWipeOut added a commit to TotalWipeOut/laminas-i18n that referenced this issue May 16, 2024
Signed-off-by: Kevin Hamilton <kevin@eagleeye.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

6 participants