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

Some translations have been lost in the new version (10.0) #1761

Closed
luisprmat opened this issue May 26, 2021 · 15 comments · Fixed by #1762
Closed

Some translations have been lost in the new version (10.0) #1761

luisprmat opened this issue May 26, 2021 · 15 comments · Fixed by #1762
Assignees
Labels
bug Something isn't working

Comments

@luisprmat
Copy link
Member

  • Laravel-Lang Version: 10.0.1
  • PHP Version: 8.0.3

Description:

Using the translations in a new project I was able to observe that some translations that were already correct have been lost.

Captura

Some of them are:

The above translations were present in the version 9.1.2 in source/en.json

Everything seems to indicate that they are the translations wrapped in Lang::get('...') and @lang directive that I referred to in this comment

Steps To Reproduce:

  • Install a new laravel project with a starter kit
  • Set some language
  • Send a reset password email
@luisprmat luisprmat added the bug Something isn't working label May 26, 2021
@caouecs caouecs self-assigned this May 26, 2021
@caouecs
Copy link
Member

caouecs commented May 26, 2021

I will do a check too

@andrey-helldar
Copy link
Member

I thought it seemed to me :-)

In the near future I will check and will check the keys from the previous version.

@andrey-helldar
Copy link
Member

@luisprmat, @caouecs, I know why this happened:

v9.1.1:
image

v10.0.1:
image

The locale.json file splitting script works like this:

  1. The contents of the locale.json file are loaded into a variable;
  2. The file for splitting is loaded, for example, nova.json;
  3. From the source file we take only those keys that are in the nova.json file;
  4. Replace the keys in the nova.json file with the original ones;
  5. Save the nova.json file;
  6. We process all other files in the same way;
  7. When finished, we get a list of all keys from the split files;
  8. From the main file, delete the keys used in the packages;
  9. Save the original file with the remaining keys.

The script removes lines like Reset Password Notification, since it is already used in nova.json.

The problem is that the Laravel framework does not have a json file with translations.

Therefore, we do not know what keys should be left in the locale.json file.

What you can do: now we have a script that scans Jetstream and Fortify projects, collects keys from them for translations and saves them to files.
I can load the Laravel Framework dependencies and do the same with it.
At the moment, this is the only option to get a complete list of these keys, since the script does not know which keys it needs to recover.

Another option: I can find all translation keys, save them to a file and merge them with the current files. It will be the same, but the project will not need to be installed when running phpunit and deploying the project locally.

@andrey-helldar
Copy link
Member

I know how to solve this problem.

A little later we will create a PR.

@andrey-helldar
Copy link
Member

Hm...

image

image

@luisprmat
Copy link
Member Author

Oh no appears in this view in Laravel 8.

@andrey-helldar
Copy link
Member

Oh no appears in this view in Laravel 8.

Yes, I have already found and deleted my unnecessary messages 😅

@andrey-helldar
Copy link
Member

I will do this:

  1. Automatic search found translation keys;
  2. Next, I will manually look at the framework files and find multi-line keys;
  3. I will add these keys to the translation file;
  4. Disable automatic search for keys in the framework;
  5. I will copy the files from version 9;
  6. Run a script that will find the translated keys for each localization from the old files and save them to the new ones;
  7. I will transfer Fix: Some translations have been lost in the new version (10.0) #1762 to the status "ready".

@andrey-helldar
Copy link
Member

Current state after searching for keys in laravel/framework project:

image

@luisprmat
Copy link
Member Author

Are they not very few? About these?

We must bear in mind that some translations do not wrap strings directly but rather variables that contain strings. For example:

  • This action is unauthorized. is passed to the $message variable through the constructor method (line 27) and later translated from this view (line 5) __($exception->getMessage() ?: 'Forbidden')

Since these strings it was always possible to translate them from previous versions I don't see why now they should disappear

@andrey-helldar
Copy link
Member

@luisprmat, should not.

The problem is that we have added translation keys nova, jetstream, fortify and others to one file.

From a code point of view, after splitting a file, it is not known which keys should remain in the main file.

This is precisely the current problem.

The screenshot above is what the parser was able to find.

This is why I will manually search for other translation keys in order to recreate the original file.

Only after that I will restore the translation of the lines that were lost during the upgrade from 9 to 10 version.

@ottenso
Copy link

ottenso commented May 26, 2021

Hey @andrey-helldar,

did I understand correctly that the error occurred only when splitting the files (from V9 to V10) and when adding new packages that also have duplicates to other packages, this error will not occur in the future? When adding new packages, the error with duplicates should not occur.

Wouldn't it be easier to optimize the splitting script (applied to V9) and apply it again to V9 (with subsequent merge to V10)? Just a quick thought from me, without understanding exactly what happened.

@andrey-helldar
Copy link
Member

@ottenso, yes it is a separation problem. I'm working on it.

@andrey-helldar
Copy link
Member

The easiest way is to return the state of the files before we added the Nova and Jetstream translations to it.

@caouecs caouecs linked a pull request Jun 2, 2021 that will close this issue
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

4 participants