Change the order of the trans method arguments

[djug]

Currently if we want to get the localized string in a particular language we need to do it like this:
trans(‘messages.welcome’, [], 'messages', $theTargetLocale);

after this changes we’ll do it like this:

trans(‘messages.welcome’, $theTargetLocale);

[lagbox]

So this would be breaking?

[djug]

@lagbox
I don't think so,

this helper method is used (elsewhere in the framework) with just the first argument ($id), so changing the order of the rest of the arguments is not supposed to break anything.

[lagbox]

If my current code has this

trans('messages.welcome', [], 'messages', $theTargetLocale);

This will break right?

[djug]

@lagbox yes,

but it is unlikely that you'll use it this way, since (in the majority of cases) you'll need to set up your locale in the config/app.php ('locale' => 'fr';) and then call trans('messages.welcome');

this commit make it possible to deal with more then one language at the same time, i.e use trans('messages.welcome'); with the primary language and use trans('messages.welcome', 'ar'); with the secondary one

[lagbox]

It doesn't matter how likely something is or not. This function exists with arguments in a particular order, if anyone in the entire world is passing more than 1 argument to that function this will break their code? So its a breaking change.

[vlakoff]

👎

$id and $parameters are the most important parameters and should stay first.

By the way, the $domain = 'messages' parameter isn't used at all (see Translator::trans() and transChoice()). Does anyone know why they were added in the first place?

[djug]

@vlakoff we need to keep $domain = 'messages' because we are implementing the Symfony\Component\Translation\TranslatorInterface interface:

public function trans($id, array $parameters = array(), $domain = null, $locale = null);

BTW, $parameters is not that important (everywhere we call trans in the framework we call it with just the key/id);

[vlakoff]

Thanks for pointing to the Symfony interface.

Maybe $parameters isn't used in the framework, but how about user code? Nevertheless, I don't see how $locale could be more important than it.

Not to mention BC break, and consistency with Translator::trans().

Why wouldn't you simply do Lang::setLocale($locale) ?

[djug]

Why wouldn't you simply do Lang::setLocale($locale) ?

please check my previous comment

[lagbox]

@GrahamCampbell For future reference, something like this should be targeting the next version since it is breaking?

I know I always like to hear how people would like to use these helper functions.

@djug I only found this function being used in the Foundation\Auth\ResetsPasswords trait, which even though is located in the core, is used by something Application level (PasswordController) . If that is the case you can define this function as you would like, and intend to use it, in your own application, before this helpers file is loaded.
Not sure where else in the framework you have found trans being called, but I will search it again. Thanks for bringing that up.

[phanan]

@djug Also, you changed the param order but left the docblock untouched…

[vlakoff]

Refs these similar issues: #2249, #11914.