laravel / ideas

Issues board used for Laravel internals discussions.
938 stars 31 forks source link

Make trans_choice() built-in into __() #1300

Open teo1978 opened 5 years ago

teo1978 commented 5 years ago

This is a feature request.

I think it's inconvenient that, in order to use pluralization, you have to use an entirely different function trans_choice() instead of the more common and nicely short-named __().

I propose that __() should automatically detect when a translation string has plural forms in it and, in that case, call trans_choice(), so you would simply always call __() both for pluralized and unpluralized translations.

Also, this form:

// time.minutes_ago => '{1} :value minute ago|[2,*] :value minutes ago'

echo trans_choice('time.minutes_ago', 5, ['value' => 5]);

is ridiculously redundant, in that you have to specify the value 5 twice.

I propose that you define a conventional placeholder name (obviously configurable), whose default could be precisely :value, so, if parameters are passed and one of them has the conventional name (value by default), it is used as the current second argument to trans_choice.

So, instead of the above, one would just call:

// time.minutes_ago => '{1} :value minute ago|[2,*] :value minutes ago'

echo __('time.minutes_ago', ['value' => 5]);

So:

  1. __() detects that the translated string has pluralization and hence handles it, via trans_choice().
  2. Additionally, since a parameter called 'value' is being passed, its value (5) is passed as the second argument to trans_choice().

Probably, __() should also accept an additional argument to specify a non-default name (i.e. other that 'value') for the parameter name that is used as the pluralization argument. So, you could use:

// time.minutes_ago => '{1} :minutes minute ago|[2,*] :minutes minutes ago'

echo __('time.minutes_ago', ['minutes' => 5], /*...whatever needed arguments*/, 'minutes');

Oh, and there should definitely be some syntax to embed the definition of the name of the "value" parameter into the definition of the phrase. Something like:

'minutes_ago' => '{1} :minutes minute ago|[2,*] :minutes minutes ago||minutes'

or perhaps more like this:

'minutes_ago|minutes' => '{1} :minutes minute ago|[2,*] :minutes minute ago'

which will tell __() that the parameter to watch for to choose the pluralization form is called 'minutes' instead of the default 'value', without having to specify it in every call.

USE CASE This happens pretty often in real life, which is why I'm surprised that this feature isn't already present.

Let's say you initially were lazy and defined a non-pluralized phrase:

// time.php
'minutes_ago' => ':value minutes ago',

and you have already used in thousands of places in your application as:

echo __('time.minutes_ago', ['value'=>$minutes]); // (*)

Now you decide that it sucks to read "0 minutes ago" or "1 minutes ago", so you decide to modify the translation definition to:

// time.php
'minutes_ago' => '{0} just now|{1} :value minute ago|[2,*] :value minutes ago'

Now, you have to go through your code and change all the calls to __() that involve this phrase with calls to trans_choice() (which additionally are redundant as noted above).

With my proposal, you wouldn't have to change them at all.

And even if the initial non-pluralized definition used a placeholder called something else other than :value, you still can make the change by only changing the translation definition (with the additional features described above) without changing anything in the calls.

thannaske commented 5 years ago

is ridiculously redundant, in that you have to specify the value 5 twice.

It's actually not you are just using the function in a way more complicated way you could. For your defense: It's not well documented. Have a look at the translation function in the framework: https://github.com/laravel/framework/blob/5.6/src/Illuminate/Translation/Translator.php#L210

As you can see whenever calling trans_choice a :count variable get's replaced per default. So you would just turn this ...

// time.minutes_ago => '{1} :value minute ago|[2,*] :value minutes ago'
echo trans_choice('time.minutes_ago', 5, ['value' => 5]);

... into this:

// time.minutes_ago => '{1} :count minute ago|[2,*] :count minutes ago'
echo trans_choice('time.minutes_ago', 5);

I propose that you define a conventional placeholder name (obviously configurable), whose default could be precisely :value, so, if parameters are passed and one of them has the conventional name (value by default), it is used as the current second argument to trans_choice.

See above: It's already done. Its conventional name is :count.

teo1978 commented 5 years ago

For your defense: It's not well documented

Yep, it's not documented at all. Not here at least: https://laravel.com/docs/5.6/localization (nor do I see there any link to a more complete reference)

Glad to hear it's already there. It seemed indeed too strange that it wasn't.

So the only part missing is making __() to automatically detect when a translation has pluralization and pass it over to trans_choice().

The part about being able to change the name of the :count placeholder is probably unnecessary, given the convention already existed before.

you are just using the function in a way more complicated way you could

Note I was following the very example provided in the docs.

thannaske commented 5 years ago

Yep, it's not documented at all. Note I was following the very example provided in the docs.

Indeed, no offense intended from my side. I'm agreeing that we should extend the __helper() to automagically detect whether its a single translation or a singluar-plural translation.

Maybe you should update your text above to make clear that this is the main proposal of this GitHub issue.

// Edit: I opened a PR to the Laravel Docs to document the magic :count placeholder. See: https://github.com/laravel/docs/pull/4468