8000 Minor BC break in TranslationUpdateCommand · Issue #26007 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Minor BC break in TranslationUpdateCommand #26007

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
pevdh opened this issue Feb 1, 2018 · 6 comments
Closed

Minor BC break in TranslationUpdateCommand #26007

pevdh opened this issue Feb 1, 2018 · 6 comments
Labels
BC Break Bug FrameworkBundle Status: Needs Review

Comments

@pevdh
Copy link
pevdh commented Feb 1, 2018 •
edited
Loading
Q A
Bug report? yes
Feature request? no
BC Break report? yes
RFC? no
Symfony version 3.4.4

According to the documentation and the translation:update description, php bin/console translation:update --force --dump-messages en is supposed to store translations in app/Resources by default.

However, in #25065 the defaultTransPath constructor argument was set to %kernel.project_dir%/translations instead of %kernel.project_dir%/app/Resources:

symfony/src/Symfony/Bundle/FrameworkBundle/Command/TranslationUpdateCommand.php

Line 54 in 8dadff2

public function __construct($writer = null, TranslationReaderInterface $reader = null, ExtractorInterface $extractor = null, $defaultLocale = null, $defaultTransPath = null, $defaultViewsPath = null)

symfony/src/Symfony/Bundle/FrameworkBundle/Resources/config/console.xml

Lines 86 to 94 in 8dadff2

<service id="console.command.translation_update" class="Symfony\Bundle\FrameworkBundle\Command\TranslationUpdateCommand">
<argument type="service" id="translation.writer" />
<argument type="service" id="translation.reader" />
<argument type="service" id="translation.extractor" />
<argument>%kernel.default_locale%</argument>
<argument>%translator.default_path%</argument>
<argument /> <!-- %twig.default_path% -->
<tag name="console.command" command="translation:update" />
</service>

symfony/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

Line 1141 in 8dadff2

$container->setParameter('translator.default_path', $config['default_path']);

symfony/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

Lines 688 to 691 in 8dadff2

->scalarNode('default_path')
->info('The default path used to load translations')
->defaultValue('%kernel.project_dir%/translations')
->end()

The former is indeed correct for sf4 projects, but directly contradicts the description of the command (and probably some documentation) in 3.4:

Usage:
  translation:update [options] [--] <locale> [<bundle>]

Arguments:
  locale                               The locale
  bundle                               The bundle name or directory where to load the messages, defaults to >>> app/Resources <<<  folder

php bin/console translation:update --force --dump-messages en app does store the translations in app/Resources/translations.

I really don't mind typing the 3 extra letters, but I thought I'd mention it either way :)
Also, not really sure if I missed anything here.
@nicolas-grekas
Copy link
Member

@yceruto maybe something you'd like to look at?

@yceruto
Copy link
Member
yceruto commented Feb 28, 2018

Will take a look soon, thanks!

@yceruto
Copy link
Member
yceruto commented Mar 27, 2018 •
edited
Loading

I can't reproduce the BC break. I just migrated one app from 3.3 to 3.4 and still this command work as expected (updating all translations in app/Resources/translations directory by default) unless you've created the translations/ directory at the root manually, meaning as of there you're starting to migrate to 4.0 structure, then it's your new default translations directory. IMHO this behavior is appropriate to migrate easily.

My suggestion, remove the translations/ dir at the root if you aren't ready to migrate to 4.0 structure yet, thus app/Resources/translations will remains the default directory.

@yceruto
Copy link
Member
yceruto commented Mar 27, 2018 •
edited
Loading

Btw, there's no option to work with translations/ dir at the root if we change the current behavior to make <kernel_root>/Resources/translations> be always the default one, because <bundle> argument doesn't accept absolute path but relative path to /Resources/* dir:

symfony/src/Symfony/Bundle/FrameworkBundle/Command/TranslationUpdateCommand.php

Lines 189 to 191 in b9a2e21

// such a bundle does not exist, so treat the argument as path
$transPaths = array($input->getArgument('bundle').'/Resources/translations');
$viewsPaths = array($input->getArgument('bundle').'/Resources/views');

@Simperfit
Copy link
Contributor

@yceruto Do you think there is something to do here or can we say that's the expected behaviour ?

@yceruto
Copy link
Member
yceruto commented Apr 23, 2019

I think this can be closed, there is no bug IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
BC Break Bug FrameworkBundle Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@javiereguiluz @nicolas-grekas @yceruto @Simperfit @pevdh @chalasr @carsonbot
0