laravel / ideas

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

Translation Service Provider Improvement Request #951

Open hifce opened 6 years ago

hifce commented 6 years ago

Hello Everyone,

Currently, Laravel gives you the option of php files under, say, /tr folder for Turkish translations or you have to create a tr.json file under lang folder.

As for php files, you have to use name.attribute to find your translation. I believe this is a little too messy, so I will just skip this option.

For json files, you can use the following format:

echo __('Hello World');

This is very readable, as you the intended message is right there. Unfortunately, one file that can hold all the translations is also messy. Json format does not allow commenting which makes it even messier.

My request is to update the translation system so that there can be multiple json files. System should cycle through them to find the intended message instead of cramming all translations into one big file that does not allow any comments to separate certain parts of the site for readability.

So json files should be created under /tr folder; not in the lang root folder.

TranslationServiceProvider should combine the json files all by itself, ideally caching them into a single file, so we could easily separate translations of distinct parts of the web site into its own files, just like how we do with blade files.

Hence, lang folder should behave like view folder internally. But the developer should not use lang files like view files as view/blade files are mentioned in a much shorter controller file, but lang files are mentioned so many times embedded inside blade files. Was I clear enough about this distinction?

Structure-wise, 'lang' files should be like 'blade' files, i.e. Files under folders.

Usage-wise, lang files should be like:

__('Translate this')

I'm not an expert developer; I'm still learning. I won't be able to develop this feature.

I hope this gets approved and implemented soon.

Thank you a lot!

Good bye!

hifce commented 6 years ago

Hello again,

A related and potentially better idea came to my mind. I wanted to quickly add it to here.

Since we mostly use translation in blade files, if translation is set to a different language, how about Laravel always look for a parallel folder structure as the blade file to find the related language file.

If for example, I am coding this file: resources/views/frontend/shop/cart.blade.php

and if the translation is on,

then Laravel could assume I will use the parallel file: resources/lang/tr/frontend/shop/cart.json

This way, we could keep the view/blade files and translation files in a similar structure and translation files could be implied, instead of "us" telling Laravel where to look or cramming them into one json file.

In short, every blade file gets a translation file.

How about that!

Anyways, thank you for considering this!

Good bye!

jrbarnard commented 6 years ago

Hi @hifce,

I think splitting out translation files into contexts makes sense (users.json and so on), however I don't they should be related to blade files as you will most likely be reusing translations in multiple files (e.g you may have a translation for 'Submit' that is used everywhere), you could of course have both defaults and view ones, but if you then decide to use that translation string elsewhere you need to / should move it).

This should be very straightforward to do and should be cached as php anyway (similar to the config php files).

I would be happy to implement this and submit a PR if that's ok with you?

Cheers, James

jrbarnard commented 6 years ago

@hifce I've created a PR into 5.5 here: https://github.com/laravel/framework/pull/22772

hifce commented 6 years ago

@jrbarnard Hello James, thanks a lot for your message!

As translations depend a lot on context as you said, and as blade views give context to translation, I would rather keep the flexibility of separating translation files per blade file (hence relate it to its context).

Sometimes, even though you are translating the same word, say payment, you may want to translate it differently in the cart then in the checkout. The word is same, but context is different, hence translation can differ. That's at least how I approach to translating my stuff.

But regardless, I am looking forward to your contribution and I'm really excited for what you've suggested. It would be really useful to be able to separate translation files per context and reduce the file sizes and still be able to use translation strings as keys.

Thank you a a lot! I really appreciate it.

hifce commented 6 years ago

@jrbarnard Hello James,

How about the attached format?

You can imagine the functionality behind it.

translationsuggestion

And if you didn't specify where to look for translation file, Laravel could assume similar structure as blade file, only under lang translation folder.

So in our case, if we didn't specify the file, then it could assume "tr.frontend.shop.checkout.address.main" as the language is set to "tr".

Thank you very much again!

jrbarnard commented 6 years ago

Hi @hifce,

I see what you mean, I think being able to specify a namespace for json translation strings would be good.

However I think a cleaner way would be to allow it to be defined via the () function, e.g: `('Product', 'shop.checkout') This would be similar to the trans() method where you specify a namespace + key. Alternatively we could potentially look at adding a method to the translator to allow you to set a root namespace: Lang::setRootNamespace('shop.checkout'); return view('...'); ` By setting a root namespace it would force any usage of the translator to look for translation strings within the specified directory structure.

The reason why I'm suggesting the above is to avoid directly linking the translator and the view, currently they do not rely on each other and so if we did need to make them link it would be significant change.

To implement either though would be a lot more change than just loading multiple files for translation files so would need to target a later release.

What do you reckon?

sisve commented 6 years ago

You're just recreating what gettext has provided for many years.

  1. comments in the translation files.
  2. existing tools for editing (https://poedit.net/ would be an example)
  3. namespaces (called these text domains, see dgettext)
  4. can extract translations from existing source files (see [xgettext])(https://www.gnu.org/software/gettext/manual/html_node/xgettext-Invocation.html)

There's a php module for this; https://secure.php.net/manual/en/book.gettext.php.

Keep in mind that _(...) (with one underline) is alias for the gettext function.

There are some quirks with gettext; it reads the current locale from the current process, so you need to use setlocale somewhere. (Perhaps a middleware that does this after reading the locale from a route segment?

jrbarnard commented 6 years ago

@sisve I agree that gettext is probably a better alternative for translation than the in built translator in Laravel, however this is more of a suggestion to improve the functionality of the Laravel json translations to bring them more in line with their php file counterparts can do (better separation & namespacing, this already exists in Laravel translations, just not in the json file version), rather than trying to recreate gettext.

nicolaaretini commented 6 years ago

Hi, I'm also interested on this topic evolution.

I'd also like to share a experience/issue I had about relying on json files in resource/lang instead of .php files. I've noticed that everything will work fine but not the validation messages. Seems like, if there's no files like ./resources/lang/[localestring]/validation.php, messages wont be retrieved, even if I have in place the corresponding ./resources/lang/[localestring].json

Piece of content of my en-GB.json (for validation):

{ ... "validation.accepted":"The :attribute must be accepted.","validation.active_url":"The :attribute is not a valid URL.","validation.after":"The :attribute must be a date after :date." .... }

Suspect is that framework is not bootstrapping/init Validator Facade correctly if it should rely on JSON files...

I tried with no success to override Validators Facade default messages with json content (json keys validation.* for the current APP::getLocale() ) at bootstrap time or via AppServiceProvider

I see also there's in Illuminate\Validation\Validator;

public function setFallbackMessages(array $messages) { $this->fallbackMessages = $messages; }

Maybe the only way is to extend Validator and setting current locale messages through this approach.....

jrbarnard commented 5 years ago

@nicolaaretini I had a brief look at this and the problem is that the validator internally uses the Translator::trans method which never gets from your json files (Illuminate\Validation\Concerns\FormatsMessages.php:57).

Based on the docs it sort of makes sense as validation messages are always referenced via key which is what the php files are meant to be for, the json files are meant to be key based on your default translation string.