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

translation extraction does not contain all strings #3117

Closed
Guite opened this issue Oct 21, 2016 · 16 comments
Closed

translation extraction does not contain all strings #3117

Guite opened this issue Oct 21, 2016 · 16 comments

Comments

@Guite
Copy link
Member

Guite commented Oct 21, 2016

Q A
Zikula Version 1.4.4-git
PHP Version 7.0.8

Expected behavior

All strings should be contained in the translation catalog.

Actual behavior

While most strings are recognised, some are not. For example in a form type class, label texts are included, but help texts are not. See this screenshot from the Legal module:

screenshot_20161021_164025

Steps to reproduce

  1. Install the Legal module
  2. Open a console in the Zikula root folder
  3. Execute php app/console translation:extract en --bundle=ZikulaLegalModule --output-format=pot
  4. Check if the resulting POT file is larger than the one which is currently in Git.
@craigh
Copy link
Member

craigh commented Oct 21, 2016

I suspect the extractor is the problem.

@craigh
Copy link
Member

craigh commented Nov 6, 2016

@Guite can you please post the file(s) that you have and also tell me where you located them in order for them to work as they do in the screen shot? (refs #3132)

@Guite
Copy link
Member Author

Guite commented Nov 6, 2016

@craigh
Copy link
Member

craigh commented Nov 6, 2016

please explain the workflow which produced these files.

Also - if you could find an example of a string that is NOT translated and provide the location/context of that compared to strings that ARE being translated, I think that would help.

I am working on some tests for the extractor which will help this I hope.

@Guite
Copy link
Member Author

Guite commented Nov 6, 2016

please explain the workflow which produced these files.

I followed zikula-modules/Pages@b9a5bcb...89ccb5c

if you could find an example of a string that is NOT translated

This works: https://github.com/zikula-modules/Legal/blob/master/Form/Type/ConfigType.php#L82
But this not: https://github.com/zikula-modules/Legal/blob/master/Form/Type/ConfigType.php#L90
(see screenshot above)

@craigh
Copy link
Member

craigh commented Nov 12, 2016

This works for me if I do

php app/console translation:extract en --bundle=ZikulaLegalModule --output-format=po

it produces translations/zikula.en.po (note the domain)

but if I do

php app/console translation:extract en --bundle=ZikulaLegalModule --output-format=po --domain=zikulalegalmodule

it produces translations/zikulalegalmodule.en.po and that catalogue doesn't contain all the strings...

so not sure how to proceed here.

@craigh craigh changed the title POT extraction does not contain all strings translation extraction does not contain all strings Nov 12, 2016
@craigh
Copy link
Member

craigh commented Nov 12, 2016

ok - I did this:

php app/console translation:extract en --bundle=ZikulaLegalModule --output-format=pot --domain=zikulalegalmodule (note I changed from .po to .pot)

A pot file includes comments to indicate where the original string is located.

This confirms that when specifying the domain that ONLY the twig files are extracted. Hence the missing strings.

then I did php app/console translation:extract en --bundle=ZikulaLegalModule --output-format=pot and this contains all the strings.

So this issue is that when specifying the domain, it doesn't extract strings from the php files. I'm not certain why this is for now.

@cmfcmf
Copy link
Contributor

cmfcmf commented Nov 12, 2016

Can you give me an example line which is not extracted when a domain is used?

@craigh
Copy link
Member

craigh commented Nov 12, 2016

Can you give me an example line which is not extracted when a domain is used?

ALL strings from php files in the requested bundle.

e.g. https://github.com/zikula-modules/Legal/blob/master/Form/Type/ConfigType.php#L82

This could be different than the behavior a couple days ago (from the OP) because I just changed some of this in my recent review of translations.

@cmfcmf
Copy link
Contributor

cmfcmf commented Nov 12, 2016

e.g. https://github.com/zikula-modules/Legal/blob/master/Form/Type/ConfigType.php#L82

It is pretty clear to me why this string is not extracted. How shall the PHP Parser, which is used to extract the strings, know that the string is of the zikulalegalmodle domain? It can't. Solution would be to use

$this->translator->trans('fosdf', [], 'zikulalegalmodule')

consistently.

Another solution would be to try to tell the extractor that the default domain inside a module folder should be the module name. No idea how this could be done right now.

@craigh
Copy link
Member

craigh commented Nov 12, 2016

Another solution would be to try to tell the extractor that the default domain inside a module folder should be the module name. No idea how this could be done right now.

this is done in the twig extractor:

// obtain translation domain from composer file
$composerPath = str_replace($this->file->getRelativePathname(), '', $this->file->getPathname());
if (isset(self::$domainCache[$composerPath])) {
$domain = self::$domainCache[$composerPath];
} else {
$scanner = new Scanner();
$scanner->scan([$composerPath], 1);
$metaData = $scanner->getModulesMetaData(true);
$domains = array_keys($metaData);
if (isset($domains[0])) {
$domain = strtolower($domains[0]);
// cache result of file lookup
self::$domainCache[$composerPath] = $domain;
} else {
$domain = 'zikula';
}
}
$domainNode = array_search($name, $this->methodNames);
$domain = $args->hasNode($domainNode) ? $args->getNode($domainNode)->getAttribute('value') : $domain;

@craigh
Copy link
Member

craigh commented Nov 14, 2016

in the php file extractor, the namespace of the file is (supposed to be) used to determine the translation domain:

if ($node instanceof \PHPParser_Node_Stmt_Namespace) {
if (isset($node->name)) {
$bundle = array_key_exists($node->name->toString(), $this->bundles) && $this->kernel->isBundle($this->bundles[$node->name->toString()]) ? $this->kernel->getBundle($this->bundles[$node->name->toString()]) : null;
if (isset($bundle) && $bundle instanceof AbstractBundle) {
$this->domain = $bundle->getTranslationDomain();
} elseif (array_key_exists($node->name->toString(), $this->bundles)) {
$this->domain = strtolower($this->bundles[$node->name->toString()]);
} else {
$this->domain = 'zikula';
}
return;
} else {
foreach ($node->stmts as $node) {
$this->enterNode($node);
}
return;
}
}

@craigh
Copy link
Member

craigh commented Nov 15, 2016

quite sure I've fixed this, but can only test with core stuff atm. please test with legal and other bundle-type modules (@cmfcmf MediaModule)

craigh added a commit that referenced this issue Nov 15, 2016
correct translation domain calculation. fixes #3117
@cmfcmf
Copy link
Contributor

cmfcmf commented Nov 15, 2016

The MediaModule can't really be used to test this, because it is (almost)
consistently using 'cmfcmfmediamodule' as third domain parameter.

Craig Heydenburg [email protected] schrieb am Di., 15. Nov. 2016,
01:48:

Closed #3117 #3117 via #3184
#3184.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#3117 (comment), or mute the
thread
https://github.com/notifications/unsubscribe-auth/ACC7RKt0vMKJ7zJ5_kbyLVwWOjn6lPcWks5q-QFogaJpZM4KdTOd
.

@craigh
Copy link
Member

craigh commented Nov 15, 2016

because it is (almost) consistently using 'cmfcmfmediamodule' as third domain parameter.

a quick look at some of the php and twig files indicates otherwise.

@rallek
Copy link
Contributor

rallek commented Nov 28, 2016

I am not shure if this is fixed already. I have to do further analysis. But not this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants