Add translation helper shell script

[justinbeaty]

Description (*)

Because in #2317 I lost track of which translation strings I introduced, I wrote a quick helper script for me to find strings used in the __ function or in XML files but not defined in a CSV file. I thought it would be useful so I fixed it up a bit and added it as shell/translations.php.

It has two modes, missing which will tell you which strings you must add to a CSV file, and unused which will print strings defined in CSV files but not apparently used anywhere in the source. Additionally, it lets you pipe in a list of files to only check, which is useful to test PRs.

Usage:  php -f translations.php -- [options]
        php -f translations.php -- missing --verbose

  missing           Display used translations strings that are missing from csv files
  unused            Display defined translations strings that are not used in templates
  --verbose         Include filename with output
  --lang <lang>     Specify which language pack to check in app/locale, default is en_US
  deprecated        Find deprecated usage of the global __() function
  --fix             Overwrite files to fix deprecated usage DO NOT RUN IN PRODUCTION!
  help              This help

Note: By default, this script will check all files in this repository. However,
      you can pipe a list of files to check for missing translations strings.
      This is useful for checking a specific commit. For example:

      # Check if last two commits may have introduced missing translations
      git diff --name-only HEAD~2 | php -f translations.php -- missing

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

1. Run php shell/translations.php missing --verbose to see php, phtml, or xml files that used non-defined translations
2. Run php shell/translations.php unused --verbose to see CSV files that have unused translations

Questions or comments

# Returns 422 unused strings
php shell/translations.php unused | wc -l

# Returns 85 missing strings
php shell/translations.php missing | wc -l

It could be possible that the code to find usage of the __ function is not perfect, such as __('Total: ' . $total) would only pick up 'Total: ', but I don't think that's used often, or shouldn't be, as the %s usage is preferred.

Contribution checklist (*)

• [x] Pull request has a meaningful description of its purpose
• [x] All commits are accompanied by meaningful commit messages
• [x] All automated tests passed successfully (all builds are green)
• [x] Add yourself to contributors list

[justinbeaty]

I did not do any sort of CSV clean up, but maybe someone else wants to take the lead on that. For example, adding missing translations or maybe removing translations that aren't used.

Edit: One warning about possibly removing unused translations, it's possible as mentioned in the previous comment that there could be some usages of calling __ with some sort of variable. For example "Google Checkout Shipping - Merchant Calculated" is not used anywhere, but it could be returned from Google's API and translated. There could be other unforeseen cases like this as well.

[elidrissidev]

Wow nice work! Will review when I get the time.

[justinbeaty]

I don't know much about github actions, but maybe this could also be used for one to check that translation strings have been included if necessary.

[fballiano]

wow this is great also for multistore development! I always like to have verborse filenames since "translations.php" is pretty generic but since it does have multiple running modes...no better name comes to my mind

[tmotyl]

have you checked if n98-magerun has sth similar already?

[justinbeaty]

@tmotyl I did look but I don't see anything like it:

  dev:translate:admin                Toggle inline translation tool for admin          
  dev:translate:export               Export inline translations                         
  dev:translate:set                  Adds a translation to core_translate table. Globally for locale
  dev:translate:shop                 Toggle inline translation tool for shop

[justinbeaty]

As mentioned in #2333, I added a new mode to find deprecated usage of the global __() function:

# list deprecated usage only
php shell/translations.php deprecated

# fix deprecated usage, DO NOT RUN IN PRODUCTION
php shell/translations.php deprecated --fix

[addison74]

This is an interesting PR and thank you for your contribution @justinbeaty. If I look at its description in the Questions or Comments section, I see that there are 422 unused strings and 85 missing strings. That is a problem. I'll take a look to see what it's all about.

For the deprecated option there is also a fix option. Running the command what is done cannot be reversed. We should evaluate if it is possible to make a backup of the files beforehand. If it is not possible, then a warning message must be mentioned to make a backup. We have to check where these changes are and find the parent directory of the files. If there are issues, either put all the files back in place, and if he uses Git he can do a revert.

[justinbeaty]

For the deprecated option there is also a fix option. Running the command what is done cannot be reversed. We should evaluate if it is possible to make a backup of the files beforehand. If it is not possible, then a warning message must be mentioned to make a backup. We have to check where these changes are and find the parent directory of the files. If there are issues, either put all the files back in place, and if he uses Git he can do a revert.

Ideally someone should not just blindly run the --fix option but should run php shell/translations.php deprecated first to see if there are any occurrences of the deprecated usage. Depending on the 3rd party module authors used, there could be zero occurrences. I think the warning is in the --help usage instructions where it says:

  --fix             Overwrite files to fix deprecated usage DO NOT RUN IN PRODUCTION!

I am not sure if we are responsible for backing up files for someone not using git and deploying directly to production. But if there's a better warning or other way to prevent accidental changes we can add it.

[justinbeaty]

As mentioned in #2429 I think a new mode can be added such as:

php shell/translations.php upgrade

This mode would allow us to change the first column in en_US csv files when we change a translation string. It would be required to add an entry to this shell script so that we can simply find/replace the changes.

In other words, take this example:

"Password must be at least of %d characters."

We can instead fix that in both columns and also put in this shell script something like:

$upgrades = array(
  "Password must be at least of %d characters." => "Password must be at least %d characters.",
);

And when the user runs php shell/translations.php upgrade it will search all other language packs for the first string to replace with the second.

Thoughts? Feedback from others who use non-english language packs is appreciated. This would be a new step users must take during upgrades.

[addison74]

Column 1 is what is used in PHP/PHTML files. Column 2 is what is used in CSV files and is displayed in Backend/Frontend.

It is obvious that we must pay attention to column 2. And then the modification should occur on column 1 of the CSV, but once we modified it must have a correspondent in the PHP/PHTML file.

PHP/PHTML FILE => COLUMN 1 CSV => COLUMN 2 CSV

To remain Column 2 then the process must be reversed.

[justinbeaty]

Let me try to re-explain what I propose.

When we want to change a string, we can do so in the PHTML files and also in both columns in the en_US csv files. This keeps everything consistent in our source code.

At the same time, we add an entry to translations.php shell script such that it will find/replace in all translation packs the old string with the new string.

So when a user upgrades to a new OpenMage version, they just have to run the script to fix what we broke in their translation files. The only negative is that they need to be aware of having to run that script.

[addison74]

I understand what you mean. I haven't tested the script yet, but I will, including in destructive mode to see what happens and how such a "mistake" can be fixed.

[sreichel]

have you checked if n98-magerun has sth similar already?

I'm pretty sure something like this already exists. If not in n98-core, maybe as addon?

However, I'll test this later :)

[sreichel]

@fballiano @Flyingmana

I'd like to have 3 further repos at OpenMage ...

• [x] openmage/dev-translations (for this PR)
• [x] openmage/dev-copyright (#2295)
• [x] openmage/dev-meta-package

These shell scripts are not needed for production ... just install them with composer require --dev openmage/dev-meta-package

[fballiano]

created the repos and set you as admin