-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Config] Adding the "info" to a missing option error messages #38176
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
src/Symfony/Bridge/Doctrine/DependencyInjection/Security/UserProvider/EntityFactory.php
Show resolved
Hide resolved
$ex = new InvalidConfigurationException(sprintf('The child node "%s" at path "%s" must be configured.', $name, $this->getPath())); | ||
$message = sprintf('The child config "%s" under "%s" must be configured.', $name, $this->getPath()); | ||
if ($child->getInfo()) { | ||
$message .= sprintf("\n\n Description: %s", $child->getInfo()); |
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 if the \n\n
is a problem. It sure does help readability.
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.
Having \n\n
looks ok to me. Maybe we can remove the space before Description:
, I would even remove the Description:
prefix.
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.
Expected value for the "%s" option: %s
instead of Description
?
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.
What do you think? Having a one liner as suggested by Nicolas seems good to me.
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.
@weaverryan Not convinced?
Fabbot failure appears false. Or at least, the referenced line was purposely changed to NOT include quotes 6 months ago: daf1c66#diff-7dae227729788bbef8c05ebb6f0f1345R210 |
Friendly ping |
Thank you @weaverryan. |
New error:
I used the missing
class
just as an example. I hit this while building a new feature in Symfony. It occurred to me that if, for example, you activate some feature in config and that feature has a required sub-key, why not print theinfo()
for that sub key to make it discoverable?