8000 Translations: incorrect domain using "t" function and named domain argument · Issue #44899 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Translations: incorrect domain using "t" function and named domain argument #44899

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.

Sign up for GitHub

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

Jump to bottom
Closed
noofaq opened this issue Jan 3, 2022 · 2 comments · Fixed by #46161
Closed

Translations: incorrect domain using "t" function and named domain argument #44899

noofaq opened this issue Jan 3, 2022 · 2 comments · Fixed by #46161
Labels
Status: Needs Review Translation

Comments

@noofaq
Copy link
noofaq commented Jan 3, 2022

Symfony version(s) affected

5.4.2

Description

Using Symfony\Component\Translation\t function and using named domain argument "translation:extract" and "debug:translations" use incorrect domain (default one messages is used).

How to reproduce

class DAMFileNameTemplate extends Constraint
{
    public function getMessage()
    {
        return t('error.dam_file_name_template_evaluation', domain: 'validators');
    }
}

Calling debug:translation reports incorrect domain:
obraz

Workaround:

class DAMFileNameTemplate extends Constraint
{
    public function getMessage()
    {
        return t('error.dam_file_name_template_evaluation', [], 'validators');
    }
}

Calling debug:translation reports correctly:
obraz

Possible Solution

No response

Additional Context

No response
@noofaq noofaq added the Bug label Jan 3, 2022
@noofaq noofaq changed the title Translation extract incorrect domain using "t" function and named domain argument Translations: incorrect domain using "t" function and named domain argument Jan 3, 2022
@fancyweb fancyweb added Translation and removed Bug labels Jan 4, 2022
@fancyweb
Copy link
Contributor
fancyweb commented Jan 4, 2022

Not a bug for me but probably an unimplemented case in the Translation PhpExtractor. Could you have a look?

@fancyweb fancyweb mentioned this issue Apr 8, 2022
Closed
1 task
@welcoMattic
Copy link
Member

We discussed with @stof & @nicolas-grekas about adding support of named arguments in PhpExtractor. It seems to require to add a new optional dependency on https://github.com/nikic/PHP-Parser to leverage AST features and avoid to continue with tokens, especially since we will probably have to add support of some other syntax (like inline variable assignation, see #42285).

@welcoMattic welcoMattic mentioned this issue Apr 25, 2022
Merged
2 tasks
@fabpot fabpot closed this as completed Oct 20, 2022
fabpot added a commit that referenced this issue Oct 20, 2022
@fabpot
feature #46161 [Translation] Add PhpAstExtractor (welcoMattic)
38678dc 
This PR was merged into the 6.2 branch.

Discussion
----------

[Translation] Add `PhpAstExtractor`

| Q             | A
| ------------- | ---
| Branch?       | 6.2
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Fix #44899, #42285, #45972, #45039
| License       | MIT
| Doc PR        | symfony/symfony-docs#17369

After discussions with `@stof` and `@nicolas`-grekas at SymfonyLive Paris 2022, it appears clear that we need a brand new PhpExtractor in Translation to support various syntax, especially PHP8 new syntax (named arguments for example).

In order to make this new extractor sustainable, performant and maintainable, it must be based on AST. That's why this PR adds a new optional dependency on [nikic/php-parser](https://github.com/nikic/PHP-Parser).

The tests suite is the same as PhpExtractorTest, in addition to new cases for newly supported syntax.

NB: I was inspired by https://github.com/php-translation/extractor design, with adaptations for Symfony.

To-do:

- [x] As suggested by `@Nyholm`, take a look at https://github.com/php-translation/extractor
- [x] Fix #45039 (could be achieved easily with AST tree)

Commits
-------

5d4a81f Add PhpAstExtractor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Status: Needs Review Translation
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Translation] Add PhpAstExtractor welcoMattic/symfony
5 participants
@fabpot @welcoMattic @noofaq @fancyweb @carsonbot
0