Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Console] Exclude empty namespaces in text descriptor#19954
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
fabpot commentedSep 17, 2016
Can you add a unit test to avoid regressions? Thanks. |
| <comment>Available commands:</comment> | ||
| <info>help</info> Displays help for a command | ||
| <info>list</info> Lists commands | ||
| <info>help</info>Displays help for a command |
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.
getColumnWidth considers aliases (descriptor:alias_command3), but we explicitly not render those.. not sure how to solve it..
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.
Can't you have the same logic in getColumnWidth to have the same "exclusion rules"?
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.
Fixed.
ro0NL commentedSep 17, 2016
fabpot commentedSep 17, 2016
Indeed, merging it in 2.7 would greatly help |
ro0NL commentedSep 17, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Would a PR do? Can you also please have a look at#19776 and decide what to do? I'd prefer this PR as a whole (all fixes can target master). |
| parent::__construct('My Symfony application','v1.0'); | ||
| $this->add(newDescriptorCommand1()); | ||
| $this->add(newDescriptorCommand2()); | ||
| $this->add(newDescriptorCommand3()); |
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.
For the record, i chose this approach on testing to include the case in other formats as well (Markdown, XML, etc.). This to avoid future regressions as well.
fabpot commentedOct 5, 2016
Can you rebase on current master? |
ro0NL commentedOct 5, 2016
Done. |
fabpot commentedMar 22, 2017
@ro0NL Sorry about that but can you rebase one last time? When done, ping me so that I can merge ASAP to avoid any problems. Thanks. |
ro0NL commentedMar 22, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@fabpot done. Replayed the before/after scenarios once more, and still all good. Waiting for travis though :) edit: green :) |
fabpot commentedMar 22, 2017
Thank you@ro0NL. |
…(ro0NL)This PR was merged into the 3.3-dev branch.Discussion----------[Console] Exclude empty namespaces in text descriptor| Q | A || --- | --- || Branch? | "master" || Bug fix? | yes || New feature? | no || BC breaks? | no || Deprecations? | no || Tests pass? | yes || Fixed tickets | comma-separated list of tickets fixed by the PR, if any || License | MIT || Doc PR | reference to the documentation PR, if any |Before:```$ bin/console doctrine doctrine:mapping:convert [orm:convert:mapping] Convert mapping information between supported formats. orm <---- router router:match Helps debug routes by simulating a path info match$ bin/console list orm [Symfony\Component\Debug\Exception\ContextErrorException] Warning: max(): Array must contain at least one element$ bin/console list generateAvailable commands for the "generate" namespace: generate:bundle Generates a bundle generate:command Generates a console command generate:controller Generates a controller```After:```$ bin/console doctrine doctrine:mapping:convert [orm:convert:mapping] Convert mapping information between supported formats. router router:match Helps debug routes by simulating a path info match$ bin/console list ormAvailable commands for the "orm" namespace: orm:convert:mapping Convert mapping information between supported formats.$ bin/console list generateAvailable commands for the "generate" namespace: generate:bundle Generates a bundle generate:command Generates a console command generate:controller Generates a controller generate:doctrine:crud Generates a CRUD based on a Doctrine entity generate:doctrine:entities Generates entity classes and method stubs from your mapping information generate:doctrine:entity Generates a new Doctrine entity inside a bundle generate:doctrine:form Generates a form type class based on a Doctrine entity```Overrules#19776 but also includes other fixes related to aliases that popped up when writing tests 👍Commits-------d5a7608 [Console] Exclude empty namespaces in text descriptor
Uh oh!
There was an error while loading.Please reload this page.
Before:
After:
Overrules#19776 but also includes other fixes related to aliases that popped up when writing tests 👍