[2.2][Console] default command #3857
Conversation
| @@ -56,7 +56,7 @@ class Application | ||
| private $autoExit; | ||
| private $definition; | ||
| private $helperSet; | ||
|
|
||
| private $defaultCommandName = null; | ||
| /** |
There was a problem hiding this comment.
missing empty line between the properties and the ocnstructor
|
There is already a place in the Application adding the |
|
and the current code is wrong: if the user does a typo when writing a command name, the default command will be called instead of warning the user about the mistake |
|
have you read the PR description? that is actually the intended behavior the idea is to be able to have applications with only one command without the need to specify the command on the command line |
|
@francodacosta using the default command when they are doing a typo ? I don't think it is a good idea. Currently (in master), we suggest possible commands in such case. Thus, arguments passed to this mistyped command would probably not match the argument expected by your default command anyway. And regarding my comment about the |
|
@stof - phpDoc definitely works with lowercase typehints, but we do have one strange inconsistency in Symfony2 where we seem to use |
|
I'm worried with the fact that I can not make clear the purpose of this change. It allows the execution of a console application with only one command without having to type the command name. I have a console application with only one command named foo I hope now you understand why it is irrelevant to talk about typos in the command name, as there is no command name The idea that any console application will have lots of commands is not always correct, and this change will serve that use case I think it is a valid use case, and I've seen more people interested in this on the mailing list. I don't mind complying with the coding standards and review suggestions, but it might be a waste of time, as you seem more interested in finding reasons not to accept the change than to incorporate it. Feel free to close the pull request if there is no interest in this change or let me know otherwise |
|
-1 IMHO I dont see this as a valid use case, It would be better coding standards if all console applications used commands even if they only have one function. Just my opinion. |
| // to execute we execute the default one | ||
| if (! is_null($this->getDefaultcommandName())) { | ||
| if (! $this->has($this->getCommandName($input))) { | ||
| $argv = $_SERVER['argv']; |
There was a problem hiding this comment.
Isnt this wrong? Command could possibly be called from a webserver instead of the command line.
|
@francodacosta I think this PR will not be mergeable anymore. Github says it is sent from |
|
yes |
|
@francodacosta so I think we need to close this PR as it cannot be merged. |
|
@francodacosta Btw, for the case where your tool only needs one command, a simpler solution is the one used by behat: overwriting the method determining the command name: https://github.com/Behat/Behat/blob/master/src/Behat/Behat/Console/BehatApplication.php#L168-176 |
This documents the way to achieve the use case requested in symfony/symfony#3857
|
Closing this PR: its code is not available anymore, I really don't like the way it is implemented, and there is already a way to build applications with a single command. I opened a PR in the doc to document the way to do it. |
For stand alone applications passing a command is mandatory but most of the times not really necessary
So instead of calling my application like this
/myapp.php mycommand --my_optionI can just write./myapp.php --my_optionthis PR allows for it while allowing to add (and execute) more than one command
You just need to add
$app->setDefaultCommandName('command:name');and if the command is not found the default one will be executedincludes unit tests and does not breack BC
it is the same as symfony/console#2, but I did the pull request on the wrong repository