topaLE
/
uptime-kuma
1
0
Fork 0
Code Issues 185 Pull Requests 26 Projects Releases 28 Wiki Activity
Labels Milestones

feat(extract-translations): add extract translations script #597

Open
bertyhell wants to merge 16 commits from bertyhell/feature/translations-extraction-script into master
Conversation 9 Commits 16 Files Changed 11
bertyhell commented 3 years ago (Migrated from github.com)
Owner

Extracts the translations from the source code
and adds the missing translations to the language files

No changes have been made to the case of the translation keys. See Discussion: https://github.com/louislam/uptime-kuma/pull/568#issuecomment-936605632

Any translations that cannot be automatically added show up as warnings.
Any translations that are in the translation files but are not used also show up as a warning.

> node ~\uptime-kuma\extra\extract-translations.js
Extraction successful with warnings:
        Cannot extract non string values in $t((this.isAdd)
        Cannot extract non string values in $t(translationPrefix + "Response")
        Cannot extract non string values in $t(translationPrefix + "Ping")
        Cannot extract non string values in $t(type)
        Language file da-DK contains keys that are not used: ["Also apply to existing monitors"]
        Language file es-ES contains keys that are not used: ["Also apply to existing monitors"]
        Language file fr-FR contains keys that are not used: ["Also apply to existing monitors"]
        Language file ja contains keys that are not used: ["Also apply to existing monitors"]
        Language file ko-KR contains keys that are not used: ["Also apply to existing monitors"]
        Language file nl-NL contains keys that are not used: ["Also apply to existing monitors"]
        Language file pl contains keys that are not used: ["Also apply to existing monitors"]
        Language file ru-RU contains keys that are not used: ["Also apply to existing monitors"]
        Language file sr-latn contains keys that are not used: ["Also apply to existing monitors"]
        Language file sr contains keys that are not used: ["Also apply to existing monitors"]
        Language file sv-SE contains keys that are not used: ["Also apply to existing monitors"]
        Language file zh-CN contains keys that are not used: ["Also apply to existing monitors"]
        Language file zh-HK contains keys that are not used: ["Also apply to existing monitors"]

After running the script the translation file changes look like this:
image

image

I've not included the output of the script in this PR to keep is small and easy for @louislam to review. Once the script is accepted @louislam or me can run the script and commit the output in a separate PR.

We might even be able to run this script as a pre-commit hook in the future using husky.

Extracts the translations from the source code and adds the missing translations to the language files No changes have been made to the case of the translation keys. See Discussion: https://github.com/louislam/uptime-kuma/pull/568#issuecomment-936605632 Any translations that cannot be automatically added show up as warnings. Any translations that are in the translation files but are not used also show up as a warning. ``` > node ~\uptime-kuma\extra\extract-translations.js Extraction successful with warnings: Cannot extract non string values in $t((this.isAdd) Cannot extract non string values in $t(translationPrefix + "Response") Cannot extract non string values in $t(translationPrefix + "Ping") Cannot extract non string values in $t(type) Language file da-DK contains keys that are not used: ["Also apply to existing monitors"] Language file es-ES contains keys that are not used: ["Also apply to existing monitors"] Language file fr-FR contains keys that are not used: ["Also apply to existing monitors"] Language file ja contains keys that are not used: ["Also apply to existing monitors"] Language file ko-KR contains keys that are not used: ["Also apply to existing monitors"] Language file nl-NL contains keys that are not used: ["Also apply to existing monitors"] Language file pl contains keys that are not used: ["Also apply to existing monitors"] Language file ru-RU contains keys that are not used: ["Also apply to existing monitors"] Language file sr-latn contains keys that are not used: ["Also apply to existing monitors"] Language file sr contains keys that are not used: ["Also apply to existing monitors"] Language file sv-SE contains keys that are not used: ["Also apply to existing monitors"] Language file zh-CN contains keys that are not used: ["Also apply to existing monitors"] Language file zh-HK contains keys that are not used: ["Also apply to existing monitors"] ``` After running the script the translation file changes look like this: ![image](https://user-images.githubusercontent.com/1710840/136442391-bafe4bf1-6a1d-4fac-91a1-9e2f2208e6ea.png) ![image](https://user-images.githubusercontent.com/1710840/136442399-04d92497-8a5b-41f6-9fb6-ef678c614655.png) I've not included the output of the script in this PR to keep is small and easy for @louislam to review. Once the script is accepted @louislam or me can run the script and commit the output in a separate PR. We might even be able to run this script as a pre-commit hook in the future using [husky](https://typicode.github.io/husky/#/).
👍 3
Saibamen commented 3 years ago (Migrated from github.com)
Owner

Please revert any changes related to package-lock.json to avoid merge conflicts

Please revert any changes related to `package-lock.json` to avoid merge conflicts
bertyhell commented 3 years ago (Migrated from github.com)
Poster
Owner

Please revert any changes related to package-lock.json to avoid merge conflicts

done
should package-lock never be committed?

> Please revert any changes related to `package-lock.json` to avoid merge conflicts done should package-lock never be committed?
👍 1
Saibamen commented 3 years ago (Migrated from github.com)
Owner

should package-lock never be committed?

Lot of changes can happen before PRs will be merged to master, so IMO it is safer to not commit lock files in PRs

> should package-lock never be committed? Lot of changes can happen before PRs will be merged to master, so IMO it is safer to not commit lock files in PRs
👍 1
Saibamen commented 3 years ago (Migrated from github.com)
Owner

@louislam can you update tests to install NPM dependencies before testing?
npm ERR! cipm can only install packages when your package.json and package-lock.json or npm-shrinkwrap.json are in sync. Please update your lock file with npm install before continuing.

@louislam can you update tests to install NPM dependencies before testing? `npm ERR! cipm can only install packages when your package.json and package-lock.json or npm-shrinkwrap.json are in sync. Please update your lock file with `npm install` before continuing.`
Saibamen commented 3 years ago (Migrated from github.com)
Owner

@bertyhell Can you please sync with current master?

@bertyhell Can you please sync with current master?
bertyhell commented 3 years ago (Migrated from github.com)
Poster
Owner

Can you please sync with current master?

done

> Can you please sync with current master? done
👍 1
Saibamen (Migrated from github.com) requested changes 3 years ago
Saibamen (Migrated from github.com) left a comment

Missing support for i18n-t tag

extra/extract-translations.js
Saibamen (Migrated from github.com) commented 3 years ago
Owner 
    // Load all ES6 module translation files into a commonJS process
```suggestion // Load all ES6 module translation files into a commonJS process ```
extra/extract-translations.js
Saibamen (Migrated from github.com) commented 3 years ago
Owner

Do we want to do it here? We already have a script for this: https://github.com/louislam/uptime-kuma/blob/master/extra/update-language-files/index.js

Do we want to do it here? We already have a script for this: https://github.com/louislam/uptime-kuma/blob/master/extra/update-language-files/index.js
Saibamen (Migrated from github.com) reviewed 3 years ago
extra/extract-translations.js
Saibamen (Migrated from github.com) commented 3 years ago
Owner

Or we can delete update-language-files script after merging this PR

Or we can delete `update-language-files` script after merging this PR
bertyhell commented 3 years ago (Migrated from github.com)
Poster
Owner

@Saibamen

Missing support for i18n-t tag

Fixed

@Saibamen > Missing support for `i18n-t` tag Fixed
👍 1
Saibamen (Migrated from github.com) approved these changes 3 years ago
bertyhell (Migrated from github.com) reviewed 3 years ago
extra/extract-translations.js
bertyhell (Migrated from github.com) commented 3 years ago
Poster
Owner

A single script seems better.

Would you like me to remove the other script in this PR? Or should a make a new PR for that?

A single script seems better. Would you like me to remove the other script in this PR? Or should a make a new PR for that?
louislam commented 3 years ago (Migrated from github.com)
Owner

Just tested. Overall is a great tool.

However, unfortunately, some keys like matrixDesc2 also added to all language files. It will be unreadable.

My suggestion would be:

  • Extract to en.js only, no need to extract to all languages.
  • npm run update-language-files have already done a good job, so since your script added all missing keys to en.js, update-language-files can fill all missing keys from en.js to all language files peacefully.

image

image

Just tested. Overall is a great tool. However, unfortunately, some keys like `matrixDesc2` also added to all language files. It will be unreadable. My suggestion would be: - Extract to `en.js` only, no need to extract to all languages. - `npm run update-language-files` have already done a good job, so since your script added all missing keys to `en.js`, `update-language-files` can fill all missing keys from `en.js` to all language files peacefully. ![image](https://user-images.githubusercontent.com/1336778/138024507-cd0c014b-98ca-4a40-828e-0d2e7e748540.png) ![image](https://user-images.githubusercontent.com/1336778/138024867-9a92d89d-b108-45a4-8418-242e6d009b61.png)
bertyhell commented 3 years ago (Migrated from github.com)
Poster
Owner

@louislam Fixed the issue. Now extracted translations in other files will default to English if it's available:
image

@louislam Fixed the issue. Now extracted translations in other files will default to English if it's available: ![image](https://user-images.githubusercontent.com/1710840/139598161-9eb3d17c-9b60-48e7-8166-f54ce2015448.png)

Reviewers

Saibamen
This pull request can be merged automatically.
You are not authorized to merge this pull request.
Sign in to join this conversation.
Reviewers
Request review
No reviewers
Saibamen
Labels
Apply labels
Clear labels
bug
Something isn't working dependencies
Pull requests that update a dependency file discussion doc
Improvements or additions to documentation duplicate
This issue or pull request already exists feature-request
New feature or request good first issue
Good for newcomers hacktoberfest hacktoberfest-accepted help help wanted
Extra attention is needed High
High Priority impossible invalid
This doesn't seem right investigating k8s Low
Low Priority Medium
Medium Priority News prerelease bug question
Further information is requested resolved Unknown wontfix
This will not be worked on
No Label bug dependencies discussion doc duplicate feature-request good first issue hacktoberfest hacktoberfest-accepted help help wanted High impossible invalid investigating k8s Low Medium News prerelease bug question resolved Unknown wontfix
Milestone
Set milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Assign users
Clear assignees
No Assignees
1 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Write Preview
Loading…
Cancel
Save
There is no content yet.
Delete Branch 'bertyhell/feature/translations-extraction-script'

Deleting a branch is permanent. It CANNOT be undone. Continue?

No
Yes