Skip to content
This repository has been archived by the owner on May 3, 2022. It is now read-only.

EZP-26430: Translation dumper to detect strings to translate in .js #721

Merged
merged 2 commits into from Nov 24, 2016

Conversation

yannickroger
Copy link
Contributor

@yannickroger yannickroger commented Nov 7, 2016

Link: https://jira.ez.no/browse/EZP-26430

Description

Some translations strings are located in JS files. This PR provides a script to automatically detect them and add them to the symfony catalog.

This extractor relies on nodejs to parse the javascript files.

Note: Translation strings using variables parameters are not detected at the moment.

Tests

Manual and unit tests.

*
*/

var walk = require( 'estree-walker' ).walk,
Copy link
Contributor

Choose a reason for hiding this comment

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

CS spaces after/before parenthesis, curly braces, ...

*/

var walk = require( 'estree-walker' ).walk,
acorn = require( 'acorn' ),
Copy link
Contributor

Choose a reason for hiding this comment

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

question: did you switch to acorn because estree-walker was not working with espree or just to follow the example in the README ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just followed the example in the README.


if( node.type && node.type == 'CallExpression' && node.callee.property.name == 'trans' ) {
result.translationsFound.push({
key: node.arguments[0].value,
Copy link
Contributor

Choose a reason for hiding this comment

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

what happens if the key and/or domain parameters are variables or any other expression ?

@yannickroger yannickroger changed the title [WIP] Translation dumper to detect strings to translate in .js [WIP] EZP-26430: Translation dumper to detect strings to translate in .js Nov 14, 2016
@yannickroger yannickroger force-pushed the ezp-26430-js_translator_dumper branch 2 times, most recently from 34e2ca2 to 21ab1fb Compare November 15, 2016 16:07
@yannickroger yannickroger changed the title [WIP] EZP-26430: Translation dumper to detect strings to translate in .js EZP-26430: Translation dumper to detect strings to translate in .js Nov 15, 2016
@@ -0,0 +1,6 @@
//Somme dummy javascript code with some translation in it.

var result1 = Y.eZ.trans('test.translation.result1', 'testdomain'),
Copy link
Contributor

Choose a reason for hiding this comment

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

},

isY = function (node) {
return node.callee.object && node.callee.object.object && node.callee.object.object.name && node.callee.object.object.name === "Y";
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you don't need && node.callee.object.object.name

},

isEz = function (node) {
return node.callee.object.property && node.callee.object.property.name && node.callee.object.property.name === "eZ";
Copy link
Contributor

Choose a reason for hiding this comment

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

same here (I guess), && node.callee.object.property.name can be removed

ast = acorn.parse(sourceCode, {}),
result = {translationsFound: []};

walk( ast, {
Copy link
Contributor

Choose a reason for hiding this comment

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

CS: no space after (


foreach ($files as $file) {
$command = 'node ' . $this->translationDumperPath . ' ' . $file;
$json = exec($command);
Copy link
Contributor

Choose a reason for hiding this comment

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

You should use shell_exec instead just in case the output is on several lines

Copy link
Contributor

Choose a reason for hiding this comment

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

and by the way, escapeshellarg on the argument

@yannickroger yannickroger force-pushed the ezp-26430-js_translator_dumper branch 2 times, most recently from 75ca75a to db40073 Compare November 17, 2016 15:53
@yannickroger
Copy link
Contributor Author

@dpobel 's feedback taken into account. Travis is fixed.

Copy link
Contributor

@dpobel dpobel left a comment

Choose a reason for hiding this comment

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

besides the last 2 comments

arguments:
- "@=service('kernel').locateResource('@eZPlatformUIBundle/Resources/public/js')"
- "@=service('kernel').locateResource('@eZPlatformUIBundle/bin/Translation/translation_dumper.js')"

Copy link
Contributor

Choose a reason for hiding this comment

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

empty line to remove

var domain;

if(isCallExpression(node) && isTransFunction(node) && isY(node) && isEz(node) && hasValues(node)) {
// Parameters not present
Copy link
Contributor

Choose a reason for hiding this comment

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

parameters is mandatory so domain is always node.arguments[2].value

@yannickroger
Copy link
Contributor Author

ping @StephaneDiot

Copy link
Member

@bdunogier bdunogier left a comment

Choose a reason for hiding this comment

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

The alias for the extractor should probably be changed.

I'll do a PR to this PR to make the resources list dynamic.


foreach ($files as $file) {
$command = 'node ' . $this->translationDumperPath . ' ' . escapeshellarg($file);
$json = shell_exec($command);
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Follow up ?

- "@=service('kernel').locateResource('@eZPlatformUIBundle/Resources/public/js')"
- "@=service('kernel').locateResource('@eZPlatformUIBundle/bin/Translation/translation_dumper.js')"
tags:
- { name: translation.extractor, alias: handlebars }
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the alias be javascript ? You already use handlebars above.

Copy link
Contributor

@StephaneDiot StephaneDiot left a comment

Choose a reason for hiding this comment

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

+1 FWIW

@yannickroger yannickroger force-pushed the ezp-26430-js_translator_dumper branch 2 times, most recently from 03f96ff to 892a31d Compare November 21, 2016 09:51
@bdunogier
Copy link
Member

Pushed a minor improvement to the dumper's implementation, as suggested by @nicolas-bastien.

@bdunogier bdunogier force-pushed the ezp-26430-js_translator_dumper branch from dd42cab to fe11825 Compare November 23, 2016 11:06
@yannickroger yannickroger merged commit a924995 into master Nov 24, 2016
@yannickroger yannickroger deleted the ezp-26430-js_translator_dumper branch November 24, 2016 09:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
4 participants