-
-
Notifications
You must be signed in to change notification settings - Fork 560
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
[console] review the usage of Command::addOption() #3174
Comments
@ao2 Totally agree on the idea of standardize this argument, and using |
@jmolivas can you do it, or should I? Thanks, |
@ao2 Feel free to take this task. |
OK, I'll look into it after #3173 gets processed, to minimize possible merge conflicts. Thanks, |
jmolivas
pushed a commit
that referenced
this issue
Apr 29, 2017
…#3174. (#3286) The second argument of Symfony\Component\Console\Command\Command::addOption() is the shortcut name of the option, and it can be set to null when not used, as reported in the documentation: http://api.symfony.com/3.1/Symfony/Component/Console/Command/Command.html#method_addOption However, currently sometimes false or an empty string are used when the option does not have a shortcut name. This could cause some confusion. Use null everywhere as the second argument when the option does not have a shortcut name.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Hi,
when working on #3092 I noticed that in some cases
false
is passed as the second argument ofCommand::addOption()
, and sometimes an empty string is passed.This made me wonder that some reader might naively end up thinking that this second argument could be the default value of the option in case the option mode is
InputOption::VALUE_OPTIONAL
(for a moment I did), but that would be wrong.The second argument is a shortcut name for the command, and the documentation says that it can be
null
if there is none:http://api.symfony.com/3.1/Symfony/Component/Console/Command/Command.html#method_addOption
A possible default value goes in the fifth argument if needed.
So what about using
null
everywhere for the shortcut argument ofaddOption()
?Just an idea.
The text was updated successfully, but these errors were encountered: