-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[ErrorRenderer] Add DebugCommand for easy debugging and testing #32504
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
Conversation
33b2036
to
dc0f057
Compare
protected function configure(): void | ||
{ | ||
$this | ||
->addArgument('format', InputArgument::OPTIONAL, sprintf('Outputs a sample in a specific format (one of %s)', implode(', ', array_keys($this->renderers)))) |
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.
Should be an option to be consistent with other commands.
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.
Here "format" doesn't have the same meaning as in other commands. Other commands use the --format
option to show the current information in another format, here the format argument shows a different information with a different meaning "Outputs an error sample in a specific format".
It could also be confusing because people would expect that with --format=json
the list of available renderers in the specified format and that is not the case.
Do you think the description of the argument should be improved to make it clearer?
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.
It is rather "I want to see an output of the error renderer associated with this format".
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.
@fabpot is this thread still a blocker?
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 think having this as argument makes sense in this case.
Thanks @ro0NL ! (Status: Needs Review) |
…nderers registration (yceruto) This PR was merged into the 4.4 branch. Discussion ---------- [ErrorRenderer] Fixed the priority order of the error renderers registration | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Spotted in #31398 and #32504 Commits ------- 710b51d Fixed the priority order of the error renderers registration
54e45b0
to
5a0845c
Compare
src/Symfony/Bundle/FrameworkBundle/Resources/config/console.xml
Outdated
Show resolved
Hide resolved
Thank you @yceruto. |
…d testing (yceruto) This PR was squashed before being merged into the 4.4 branch (closes #32504). Discussion ---------- [ErrorRenderer] Add DebugCommand for easy debugging and testing | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | -  This command helps you test built-in renderers and others provided by third-party packages (thinking API-Platform, FOSRestBundle, etc.) without requiring the TwigBundle preview mechanism. <del>This also fixes a bug in `ErrorRendererPass` related to the order of priorities.</del> Commits ------- 97c8968 [ErrorRenderer] Add DebugCommand for easy debugging and testing
This command helps you test built-in renderers and others provided by third-party packages (thinking API-Platform, FOSRestBundle, etc.) without requiring the TwigBundle preview mechanism.
This also fixes a bug inErrorRendererPass
related to the order of priorities.