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

Fix: Some translations have been lost in the new version (10.0) #1762

Merged
merged 23 commits into from Jun 2, 2021
Merged

Fix: Some translations have been lost in the new version (10.0) #1762

merged 23 commits into from Jun 2, 2021

Conversation

andrey-helldar
Copy link
Member

@andrey-helldar andrey-helldar commented May 26, 2021

  • Recover localization files from version 9;
  • Write and run a script that will find untranslated keys in recovered files and take the translation from files in the locales/{locale}/packages folder;
  • Delete the locales/{locale}/packages folders;
  • Change the key-value object to an array of values in the source/packages folder;
  • Find translatable lines calls in a laravel/framework version 6 project and save them to source/en.json file;
  • Find translatable lines calls in a laravel/framework version 7 project and save them to source/en.json file;
  • Find translatable lines calls in a laravel/framework version 8 project and save them to source/en.json file;
  • Update script app/excludes.php;
  • Update script app/keys.php;
  • Update script app/packages.php;
  • Update script app/status.php;
  • Delete script app/split-packages.php;
  • Update tests.

@andrey-helldar
Copy link
Member Author

andrey-helldar commented May 26, 2021

@caouecs, can you linking this PR with #1761? I have no rights to do this.

image

@luisprmat
Copy link
Member

Checking internally Laravel code it seems the only translation that does not match the search patterns that you handle is

  • This action is unauthorized.

Captura
(because it belongs to an exception message: This action is unauthorized. ) this key is later translated from view src/Illuminate/Foundation/Exceptions/views/403.blade.php with the helper __(...)

__($exception->getMessage() ?: 'Forbidden')

I consider that this key needs to be added manually.

Multi-line keys apparently only exist for notification mails subcopy slot

...
@lang(
    "If you’re having trouble clicking the \":actionText\" button, copy and paste the URL below\n".
    'into your web browser:',
    [
        'actionText' => $actionText,
    ]
) <span class="break-all">[{{ $displayableActionUrl }}]({{ $actionUrl }})</span>
...

@andrey-helldar
Copy link
Member Author

@luisprmat, @caouecs, everything is in order with the Russian language. Can you check fr and es?

I grabbed the file from the #1335 (before Jetstream was added), restored the files from version 9.1.2, and ran php app/keys.php.

@andrey-helldar andrey-helldar marked this pull request as ready for review May 27, 2021 09:41
@sotten
Copy link
Contributor

sotten commented May 27, 2021

@andrey-helldar I have checked de.json and de/jetstream.json, the translations are correct. I did not check for missing translation keys.

locales/es/es.json Outdated Show resolved Hide resolved
@andrey-helldar andrey-helldar marked this pull request as draft May 27, 2021 13:55
locales/es/es.json Show resolved Hide resolved
@caouecs caouecs added this to the v10.1 milestone May 28, 2021
Andrey Helldar added 2 commits June 1, 2021 19:07
…2-46

# Conflicts:
#	locales/pt_BR/packages/cashier.json
#	locales/pt_BR/packages/jetstream.json
#	locales/pt_BR/packages/nova.json
#	locales/pt_BR/packages/spark-paddle.json
#	locales/pt_BR/packages/spark-stripe.json
#	locales/tr/packages/cashier.json
#	locales/tr/packages/jetstream.json
#	locales/tr/packages/nova.json
#	locales/tr/packages/spark-paddle.json
#	locales/tr/packages/spark-stripe.json
#	locales/tr/tr.json
@andrey-helldar andrey-helldar marked this pull request as ready for review June 1, 2021 16:36
@andrey-helldar
Copy link
Member Author

@Laravel-Lang/laravel-lang, guys, check this one PR, please.

@andrey-helldar andrey-helldar requested review from sotten and a team June 1, 2021 16:44
Copy link
Member

@luisprmat luisprmat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's see if I understand your idea.

  • In the source folder it only contains arrays with the keys (only keys without translations) used in the {locale}.json base and all packages including repetitions if necessary (for example breeze and jetstream share several keys therefore they appear in both files).
  • In locales folder a single file locales/{locale}/{locale}.json with all mixed keys with translations and a unique translation.

I think it is a great idea!! Just a question, Are the keys within the packages still searched automatically?

So I think an excellent job has been done, ¡APPROVED!

@andrey-helldar
Copy link
Member Author

@luisprmat:

In the source folder it only contains arrays with the keys

Yes

In locales folder a single file

Yes

Are the keys within the packages still searched automatically?

Auto-search (these projects do not have json and php files with translations):

  • laravel/breeze
  • laravel/fortify
  • laravel/jetstream

Manually copying files (these projects have json or php files with translations):

  • laravel/cashier
  • laravel/jetstream
  • laravel/fortify
  • laravel/nova
  • laravel/spark-paddle
  • laravel/spark-stripe

Copy link
Member

@caouecs caouecs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some questions on code

app/main/Processors/Excludes.php Show resolved Hide resolved
app/main/Processors/Processor.php Show resolved Hide resolved
@sotten
Copy link
Contributor

sotten commented Jun 1, 2021

@andrey-helldar excellent job! I skimmed de.json and de_CH.json and could not find any errors.
APPROVED

@caouecs caouecs linked an issue Jun 2, 2021 that may be closed by this pull request
@andrey-helldar
Copy link
Member Author

Just in case, I'll write how I worked with translations:

  1. Copied all localization files from version 9;
  2. Launched a script that found all json files in the localization folder;
  3. The script combined all the keys into one array with the uniqueness check;
  4. Saved the resulting array to the {locale}/{locale}.json file;
  5. Removed packages folders from localizations.

@caouecs
Copy link
Member

caouecs commented Jun 2, 2021

I merge this pull request, and I create a v10.1

@caouecs caouecs merged commit ae8b99f into Laravel-Lang:master Jun 2, 2021
@caouecs
Copy link
Member

caouecs commented Jun 2, 2021

thank you

@mohamedsabil83
Copy link
Member

Thank you @andrey-helldar and the rest for the great work and suggestions have done.
Also, I apologize for my absence the past few days due to my illness.

@andrey-helldar
Copy link
Member Author

@mohamedsabil83, thank you! I hope you are doing well with your health now.

@mohamedsabil83
Copy link
Member

Thank you for asking @andrey-helldar. I am currently recovering and will be back to work soon.

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 this pull request may close these issues.

Some translations have been lost in the new version (10.0)
5 participants