search

php-gettext / Gettext

PHP library to collect and manipulate gettext (.po, .mo, .php, .json, etc)
MIT License
687 stars 134 forks source link

Scanning for messages in a specific domain #179

Closed briedis closed 6 years ago

briedis commented 6 years ago

So now if I scan files and do not set the domain of the translation file, only default domain messages are scanned.

If I set a domain for the translation file, that domain messages are scanned, but also non-domain messages are scanned. This is a bummer, because I cannot get messages only from a certain domain and there is no way.

I suggest adding an option for the extractors which would allow scanning only domain messages.

This should be an easy fix, all the magic is happening here: Utils/FunctionsScanner.php:110

I'd be willing to add this functionality and update tests.

What do you think @oscarotero ?

oscarotero commented 6 years ago

Mmm, it makes sense. Currently, extractors mix messages from a specific domain with messages with no domain (or not specified in the gettext function). So an option to make the extraction more strict is a good idea.

briedis commented 6 years ago

Would it make sense to make this a breaking change and change the code so it scans for only the specified domain messages? For me it seems that this would be the expected behaviour and I can't see a case where you would want to scan for a specific domain and include the non-domain messages.

Perhaps, a new functionality could scan for all messages and sort them in separate translation files indexed by matching domains.

oscarotero commented 6 years ago

I don't want to release a major version only for this change, but it can be included when v5.x is released.

This behavior was choosen because extracting translations, the lack of domain in the gettext function does not means the translation has not domain. It's usual to define a default domain in the templates and use simply __('string'), instead d__('domain', 'string').

A new option to make this proccess more strict is a good way to include this new use case without break backward compatibility.

briedis commented 6 years ago

Alright, then I'll make it as an option.

Maybe you have a suggestions which tests should I update?

oscarotero commented 6 years ago

Currently there's three tests with phpcode (phpcode, phpcode2 and phpcode3), but none of them is valid to test this new feature, because there's no enough domain/without domain gettext functions mixed. I would create a new phpcode4 to test this. You can create the files with the expected results using some utils stored here: https://github.com/oscarotero/Gettext/blob/master/tests/AbstractTest.php#L67

briedis commented 6 years ago

Related pull request: https://github.com/oscarotero/Gettext/pull/181

briedis commented 6 years ago

Merged, thanks!