Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[WIP][FrameworkBundle] Add debug autoconfigure command#28730
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
nicolas-grekas left a comment
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.
Thanks for working on this, here is a round of comments.
| } | ||
| $tableRows[] = array('Public', $autoconfiguredInstanceofItem->isPublic() ? 'yes' : 'no'); | ||
| $tableRows[] = array('Shared', $autoconfiguredInstanceofItem->isShared() ? 'yes' : 'no'); | ||
| $tableRows[] = array('Autowired', $autoconfiguredInstanceofItem->isAutowired() ? 'yes' : 'no'); |
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.
The only things that can be set by autoconfiguration are tags, method calls and bindings.
There is no need to list anything else.
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.
I removed Public, Shared and Autowired and I add bindings
| } | ||
| if ($tags && array_values($tags) !== array(array(array()))) { | ||
| $tableRows[] = array('Tags attributes', $this->dumpTagsAttributes($tags)); |
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.
If there are any attributes, they should be listed next to their corresponding tags.
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.
If there are many attributes, the command displays like that
--------------- -------------------------- Option Value --------------- -------------------------- Tag kernel.reset Tag attribute [ [ "method" => "reset" ] ] Tag kernel.reset2 Tag attribute [ [ "method" => "reset2" ] ] --------------- --------------------------| /** @var ChildDefinition $autoconfiguredInstanceofItem */ | ||
| foreach ($autoconfiguredInstanceofItems as $key => $autoconfiguredInstanceofItem) { | ||
| $tags = $autoconfiguredInstanceofItem->getTags(); | ||
| $tableRows = array(); |
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.
maybe we can get rid of the$tableRows array by usingaddRow() andrender(), as this is formaster. IMO its more readable afterwards
docs:https://symfony.com/doc/current/components/console/helpers/table.html#modifying-rendered-tables
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.
I'm not sure it's more readable. To have the same style that the others commands, it is necessary to configure the table.
diff --git a/src/Symfony/Bundle/FrameworkBundle/Command/DebugAutoconfigurationCommand.php b/src/Symfony/Bundle/FrameworkBundle/Command/DebugAutoconfigurationCommand.phpindex bffdd32751..6d80649634 100644--- a/src/Symfony/Bundle/FrameworkBundle/Command/DebugAutoconfigurationCommand.php+++ b/src/Symfony/Bundle/FrameworkBundle/Command/DebugAutoconfigurationCommand.php@@ -12,6 +12,7 @@ namespace Symfony\Bundle\FrameworkBundle\Command; use Symfony\Component\Console\Command\Command;+use Symfony\Component\Console\Helper\Table; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption;Stash this hunk [y,n,q,a,d,j,J,g,/,e,?]? y@@ -91,26 +92,30 @@ EOF /** @var ChildDefinition $autoconfiguredInstanceofItem */ foreach ($autoconfiguredInstanceofItems as $key => $autoconfiguredInstanceofItem) {- $tableRows = array();+ $section = $output->section();+ $table = new Table($section);+ $table->setStyle(Table::getStyleDefinition('symfony-style-guide'));+ $table->addRow(array('Option', 'Value')); foreach ($autoconfiguredInstanceofItem->getTags() as $tag => $tagAttributes) {- $tableRows[] = array('Tag', $tag);+ $table->addRow(array('Tag', $tag)); if ($tagAttributes !== array(array())) {- $tableRows[] = array('Tag attribute', $this->dumpTagAttribute($tagAttributes));+ $table->addRow(array('Tag attribute', $this->dumpTagAttribute($tagAttributes))); } } if ($autoconfiguredInstanceofItem->getMethodCalls()) {- $tableRows[] = array('Method call', $this->dumpMethodCall($autoconfiguredInstanceofItem));+ $table->addRow(array('Method call', $this->dumpMethodCall($autoconfiguredInstanceofItem))); } if ($autoconfiguredInstanceofItem->getBindings()) {- $tableRows[] = array('Bindings', $this->dumpBindings($autoconfiguredInstanceofItem));+ $table->addRow(array('Bindings', $this->dumpBindings($autoconfiguredInstanceofItem))); } $io->writeln(sprintf("Autoconfiguration for \"%s\"\n==============================================", $key)); $io->newLine();- $io->table(array('Option', 'Value'), $tableRows);+ $table->render();+ $io->newLine(); } }OskarStark commentedOct 6, 2018 • 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.
Could you please post a screenshot of the output? EDIT: Please add it to the PR header |
35a8f47 to09ea820Compareaaa2000 commentedOct 9, 2018
@OskarStark Have you seen the output of command in the PR header, it requires to unfold. Do you prefer a screenshot ? |
| @@ -0,0 +1,9 @@ | |||
| <?php | |||
| namespace Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\DebugAutoconfigurationBundle; | |||
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.
Maybe, I can use the TestBundle instead of create a new bundle ?
4074a0d to00fb8aeCompareOskarStark commentedOct 13, 2018
Ah i missed it 😊 |
ro0NL left a comment
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.
not sure we need a new command, feels like it should be part ofdebug:container --autoconfiguration instead. Re-building and YAML dumping the container here looks suspicious to me.
Alternatively extend fromContainerDebugCommand likeDebugAutowiringCommand also does.
aaa2000 commentedNov 1, 2018
Ok, I don't like it too but I didn't see other way to display method calls and bindings. |
Uh oh!
There was an error while loading.Please reload this page.
Close#26295
Replace the pull request#26745.
Creation of a new command as proposed in the comment#26745 (comment) to show the full template of the autoconfigure of interfaces/classes
Output of the command based on modified symfony-demo application to have binding autoconfiguration, multiple tags attributes and multiple method calls