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] Fix TranslationNodeVisitor with constant domain #53357

Merged
merged 1 commit into from
Jan 23, 2024

Conversation

VincentLanglet
Copy link
Contributor

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #...
License MIT

Related to #53356 but fixing only the twig part.

@VincentLanglet
Copy link
Contributor Author

VincentLanglet commented Jan 2, 2024

Not sure if I should fix the fail check for code I didn't touch ?

@OskarStark OskarStark changed the title [Translation] Fix TranslationNodeVisitor with constant domain [Translation] Fix TranslationNodeVisitor with constant domain Jan 2, 2024
@@ -158,6 +158,19 @@ private function getReadDomainFromNode(Node $node): ?string
return $node->getAttribute('value');
}

if (
$node instanceof FunctionExpression
Copy link
Member

Choose a reason for hiding this comment

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

Based on the updated fixture I would have expected a FilterExpression here. But that makes me think that we probably need to provide the fix for the trans tag, t() function as well as the trans filter.

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 added test and they are now supported 52b111a

@VincentLanglet VincentLanglet requested a review from xabbuh January 3, 2024 08:55
@VincentLanglet
Copy link
Contributor Author

@xabbuh Should I fix the CI ? The error (even from fabbot.io) seems unrelated to my changes, and I'm affraid about conflict during futur merge if I start fixing them...

@xabbuh
Copy link
Member

xabbuh commented Jan 11, 2024

@VincentLanglet You can safely ignore things suggested by fabbot that haven't been touched by your changes.

@VincentLanglet
Copy link
Contributor Author

@VincentLanglet You can safely ignore things suggested by fabbot that haven't been touched by your changes.

Thanks ; I think it's ready then :)

@nicolas-grekas nicolas-grekas force-pushed the translation-twig-visitor branch from b53a9be to d14795d Compare January 23, 2024 13:16
@nicolas-grekas
Copy link
Member

Thank you @VincentLanglet.

@nicolas-grekas nicolas-grekas merged commit c32e249 into symfony:5.4 Jan 23, 2024
nicolas-grekas added a commit that referenced this pull request Jan 29, 2024
…actor (VincentLanglet)

This PR was squashed before being merged into the 6.4 branch.

Discussion
----------

[Translation] Fix constant domain resolution in PhpAstExtractor

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #53356
| License       | MIT

Similar to #53357

Closes #53356

Commits
-------

c43f6c0 [Translation] Fix constant domain resolution in PhpAstExtractor
@fabpot fabpot mentioned this pull request Jan 30, 2024
This was referenced Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants