Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[DebugBundle] Added 'theme' option to change the color of dump() when rendered inside templates#29528
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
[DebugBundle] Added 'theme' option to change the color of dump() when rendered inside templates#29528
Uh oh!
There was an error while loading.Please reload this page.
Conversation
src/Symfony/Bundle/DebugBundle/DependencyInjection/Configuration.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Bundle/DebugBundle/DependencyInjection/DebugExtension.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
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.
LGTM with some minor comments.
| ->end() | ||
| ; | ||
| if (class_exists(HtmlDumper::class) &&method_exists(HtmlDumper::class,'setTheme')) { |
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 be simplified:if (method_exists(HtmlDumper::class, 'setTheme')) {
| ->addMethodCall('setMinDepth',array($config['min_depth'])) | ||
| ->addMethodCall('setMaxString',array($config['max_string_length'])); | ||
| if (class_exists(HtmlDumper::class) &&method_exists(HtmlDumper::class,'setTheme')) { |
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 ('dark' !== $config['theme'] && method_exists(HtmlDumper::class, 'setTheme')) {
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'dark' !== $config['theme'] will not launch an undefinex index notice with var dumper 4.1?
In that case, the Configuration class will not initialize the $config['theme'] index because 'setTheme' doesn't exists.
it wouldn't be better to set something like this?:
if (isset($config['theme'] && 'dark' !== $config['theme'] && method_exists(HtmlDumper::class, 'setTheme')) {
Or maybe leave it just as:if (method_exists(HtmlDumper::class, 'setTheme')) {
as it appears in the Configuration class.
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.
Correct.if (method_exists(HtmlDumper::class, 'setTheme') && 'dark' !== $config['theme']) { then!
fabpot 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.
with a minor comment
| ->children() | ||
| ->enumNode('theme') | ||
| ->info('Changes the color of the dump() output when rendered directly on the templating. "dark" (default) or "light"') | ||
| ->example('"dark"') |
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 otherexample calls we have do not wrap strings, should probably be->example('dark').
feb6f34 toc0bb3a7Comparefabpot commentedJan 1, 2019
Something went bad when merging, can you re-push your work here? Sorry about that. |
c0bb3a7 tofeb6f34Comparedem3trio commentedJan 1, 2019
I've made agit push --force it's that enough? If you need anything else, tell me. |
fabpot commentedJan 2, 2019
It's not enough. You need to execute |
feb6f34 toa0db35aComparedem3trio commentedJan 2, 2019
created new origin "sf" (symfony/symfony) I hope i've done well this time |
… rendered inside templates
a0db35a to91e8057Comparefabpot commentedJan 3, 2019
Thank you@dem3trio. |
… of dump() when rendered inside templates (dem3trio)This PR was squashed before being merged into the 4.3-dev branch (closes#29528).Discussion----------[DebugBundle] Added 'theme' option to change the color of dump() when rendered inside templates| Q | A| ------------- | ---| Branch? | master for features| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| License | MITAdded a html_dumper_theme option config to use the new HtmlDumper light theme when using dump() inside templates#SymfonyConHackday2018Commits-------91e8057 [DebugBundle] Added 'theme' option to change the color of dump() when rendered inside templates
Added a html_dumper_theme option config to use the new HtmlDumper light theme when using dump() inside templates
#SymfonyConHackday2018