Skip to content

[FrameworkBundle] Add --no-prefix option to translation:update #20432

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

Merged

Conversation

chalasr
Copy link
Member

@chalasr chalasr commented Nov 7, 2016

Q A
Branch? master
Bug fix? yes
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #20044
License MIT
Doc PR n/a

This adds an option --no-prefix to the translation:update command, allowing to use an empty string as prefix. I guess it should be treated as a feature as it adds a new option to the command, but it indeed fixes the bug reported in #20044 (yeah, really this time).

Copy link
Member

@javiereguiluz javiereguiluz left a comment

Choose a reason for hiding this comment

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

👍

Adding this option looks like the only possible alternative to solve this issue without creating more troubles.

$extractor->setPrefix(null === $prefix ? '' : $prefix);

if (false === $input->getOption('no-prefix')) {
$extractor->setPrefix(false === $input->getOption('no-prefix') ? $input->getOption('prefix') : '');
Copy link
Member

Choose a reason for hiding this comment

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

The false === $input->getOption('no-prefix') condition is redundant here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops :) it is fixed now

@@ -39,6 +39,7 @@ protected function configure()
new InputArgument('locale', InputArgument::REQUIRED, 'The locale'),
new InputArgument('bundle', InputArgument::OPTIONAL, 'The bundle name or directory where to load the messages, defaults to app/Resources folder'),
new InputOption('prefix', null, InputOption::VALUE_OPTIONAL, 'Override the default prefix', '__'),
new InputOption('no-prefix', null, InputOption::VALUE_NONE, 'Should the prefix be an empty string'),
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we could reword this description as: If set, no prefix is added to the translations.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated, thank you @javiereguiluz

@chalasr chalasr force-pushed the framework/translation-upd-no-prefix-opt branch 3 times, most recently from 6e318d5 to 53ed38f Compare November 7, 2016 14:24
$extractor = $this->getContainer()->get('translation.extractor');
$extractor->setPrefix(null === $prefix ? '' : $prefix);
$extractor->setPrefix(false === $input->getOption('no-prefix') ? $input->getOption('prefix') : '');
Copy link
Member

Choose a reason for hiding this comment

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

I think $input->getOption('no-prefix') ? '' : $input->getOption('prefix') will be more readable.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 done

@chalasr chalasr force-pushed the framework/translation-upd-no-prefix-opt branch from 53ed38f to 914e3a9 Compare November 7, 2016 19:30
Remove ending dot from option description for consistency
@chalasr chalasr force-pushed the framework/translation-upd-no-prefix-opt branch from 914e3a9 to b5a1584 Compare November 7, 2016 19:38
@Simperfit
Copy link
Contributor

👍

1 similar comment
@aitboudad
Copy link
Contributor

👍

@chalasr
Copy link
Member Author

chalasr commented Nov 10, 2016

Is this planed for 3.2? An unsuccessful fix being released and finally reverted, I think the earlier possible would be the best.

@javiereguiluz javiereguiluz added this to the 3.2 milestone Nov 10, 2016
@xabbuh
Copy link
Member

xabbuh commented Nov 10, 2016

👍

@fabpot
Copy link
Member

fabpot commented Nov 11, 2016

Thank you @chalasr.

@fabpot fabpot merged commit b5a1584 into symfony:master Nov 11, 2016
fabpot added a commit that referenced this pull request Nov 11, 2016
…date (chalasr)

This PR was merged into the 3.2-dev branch.

Discussion
----------

[FrameworkBundle] Add --no-prefix option to translation:update

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #20044
| License       | MIT
| Doc PR        | n/a

This adds an option `--no-prefix` to the `translation:update` command, allowing to use an empty string as prefix. I guess it should be treated as a feature as it adds a new option to the command, but it indeed fixes the bug reported in #20044 (yeah, really this time).

Commits
-------

b5a1584 [FrameworkBundle] Add --no-prefix option to translation:update
@fabpot fabpot mentioned this pull request Nov 17, 2016
@chalasr chalasr deleted the framework/translation-upd-no-prefix-opt branch May 10, 2017 16:44
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.

7 participants