8000 [FrameworkBundle] Enable 'debug:translation' command to inspect directories by xelaris · Pull Request #13340 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[FrameworkBundle] Enable 'debug:translation' command to inspect directories #13340

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
wants to merge 1 commit into from
Closed

[FrameworkBundle] Enable 'debug:translation' command to inspect directories #13340

wants to merge 1 commit into from

Conversation

xelaris
Copy link
Contributor
@xelaris xelaris commented Jan 9, 2015
Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #12273
License MIT
Doc PR

This enables not only to inspect Bundles, but also directories, like the app/ directory for instance.

I'm not sure about the branch, strictly speaking it's a new feature, but in some way a bug fix too.

@fabpot
Copy link
Member
fabpot commented Jan 9, 2015

It adds a new flag, so this is definitely a new feature and it will be merged in 2.7 instead.

fabpot
fabpot reviewed Jan 9, 2015
View reviewed changes
src/Symfony/Bundle/FrameworkBundle/Command/TranslationDebugCommand.php
@@ -45,6 +45,7 @@ protected function configure()
new InputOption('domain', null, InputOption::VALUE_OPTIONAL, 'The messages domain'),
new InputOption('only-missing', null, InputOption::VALUE_NONE, 'Displays only missing messages'),
new InputOption('only-unused', null, InputOption::VALUE_NONE, 'Displays only unused messages'),
new InputOption('path', 'p', InputOption::VALUE_NONE, 'Treats bundle name as path'),
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need a new option? What about looking for a path if the bundle does not exist instead?

@xelaris
[FrameworkBundle] Enable translation debugging in directories
37d53ff 
This enables not only to inspect Bundles, but also directories, like
the app/ directory for instance.
@xelaris xelaris force-pushed the debug-translation branch from f2c82e5 to 37d53ff Compare January 9, 2015 09:42
@xelaris
Copy link
Contributor Author
xelaris commented Jan 9, 2015

Thanks for the feedback @fabpot. I changed it according to your suggestion.

@xelaris xelaris changed the title [FrameworkBundle] Added 'path' option to 'debug:translation' command [FrameworkBundle] Enable 'debug:translation' command to inspect directories Jan 9, 2015
@fabpot
Copy link
Member
fabpot commented Jan 9, 2015

Looks good to me now

👍

@stof
Copy link
Member
stof commented Jan 9, 2015

translation:update should also be updated for consistency

@xelaris
Copy link
Contributor Author
xelaris commented Jan 9, 2015

Nice hint @stof. I had a look at the translation:update command now and found out, that it already supports the processing of the app/ directory, but in a different way (see #11254).
I agree, both commands should be consistent. That would mean the bundle param of the translation:debug command should also be optional and default to the app kernel dir. Contrary, the translation:update command should fallback to treat the bundle param as a directory then.
After a quick look on both commands, I wonder if it would be reasonable to extract an abstract command. What do you think? @stof @fabpot I'm also in favor of closing this PR and open a new one for 2.7 to update both commands.

@fabpot
Copy link
Member
fabpot commented Jan 16, 2015

@xelaris Opening a new one for 2.7 and make both commands behave the same looks like the way to go. I would not create an abstract class though. Thanks.

@xelaris xelaris mentioned this pull request Jan 18, 2015
Merged
@xelaris
Copy link
Contributor Author
xelaris commented Jan 18, 2015

closed in favor of #13443

@xelaris xelaris closed this Jan 18, 2015
aitboudad added a commit that referenced this pull request Apr 6, 2015
@aitboudad
feature #13443 [Translation][Command][FrameworkBundle] Enable transla…
6CC5
cc13ae4 
…tion debugging in directories (xelaris)

This PR was merged into the 2.7 branch.

Discussion
----------

[Translation][Command][FrameworkBundle] Enable translation debugging in directories

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #12273
| License       | MIT
| Doc PR        |

This PR follows up #13340 and enables not only to inspect Bundles, but also directories, like the `app/` directory.

Additionally it harmonizes the TranslationDebugCommand and TranslationUpdateCommand to expect an optional bundle name or a directory and fall back to kernel root dir if none of them is given.

Commits
-------

2662244 [FrameworkBundle] Enable translation debugging in directories
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
FrameworkBundle
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@xelaris @fabpot @stof
0