-
-
Notifications
You must be signed in to change notification settings - Fork 356
[Translator] Revert Fix changing dump directory using AssetMapper #1913
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
[Translator] Revert Fix changing dump directory using AssetMapper #1913
Conversation
$config['dump_directory'] => 'var/translations', // Keeping for BC, see https://github.com/symfony/ux/pull/1904#issuecomment-2162066941 | ||
$config['dump_directory'] => '@app/translations', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You will only have 'var/translations'
or '@app/translations'
in the array, since the key is the same for both values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, when we tested it worked because we had it the other way around.
There's no way to fix this because even with
asset_mapper' => [
'paths' => [
__DIR__.'/../../assets/dist' => '@symfony/ux-translator',
$config['dump_directory'] => '@app/translations',
'%kernel.project_dir%/var/translations' => 'var/translations',
],
],
the keys will be the same when running the default conf.
This reverts commit a27cfc8
4373448
to
e551cff
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep and it happens don't worry...
(some even say only the best of the best do this 😇 😆 )
... and we should have been more attentive.
Thank you :)
Thank you @maelanleborgne. |
This PR introduced a bug : when using 'var/translations' in the
importmap.php
file instead of a relative path, it would break the twigimportmap()
function.This PR aims to rectify this regression by keeping a 'var/translations' namespace pointing to the dump_directory. If this is not accepted, the previous PR should be reverted to prevent users using 'var/translations' in thereimportmap.php
to face this issue, but the documentation should still be updated to display an example using the relative path instead of a misleading namespace.This PR reverts the change. A word in the doc to say that the
dump_directory
conf is not compatible with asset_mapper would be nice.