Handle subcommands in fewer places.

Rework Shell::runCommand() so it doesn't require parameters to be
duplicated. This has the side benefit of making option parsing have
a simpler interface.

Update related tests, and code. CompletionShell had a silly parser
definition that I've corrected.

If a command's option parser fails, the error is now shown before the
help message. Hopefully this gives the end user some idea on what they
need to change when trying to call the shell.

This change continues to keep sketchy logic around for task's being
called through execute(). I'm going to address this in my next set of
changes.

src/Console/Command/CompletionShell.php

@@ -107,8 +107,6 @@ class CompletionShell extends Shell {
 			'help' => __d('cake_console', 'Output a list of available commands'),
 			'parser' => [
 				'description' => __d('cake_console', 'List all availables'),
-				'arguments' => [
-				]
 			]
 		])->addSubcommand('subcommands', [
 			'help' => __d('cake_console', 'Output a list of available subcommands'),
@@ -117,7 +115,7 @@ class CompletionShell extends Shell {
 				'arguments' => [
 					'command' => [
 						'help' => __d('cake_console', 'The command name'),
-						'required' => true,
+						'required' => false,
 					]
 				]
 			]

src/Console/ConsoleOptionParser.php

@@ -463,9 +463,12 @@ class ConsoleOptionParser {
  * @return Array [$params, $args]
  * @throws \Cake\Console\Error\ConsoleException When an invalid parameter is encountered.
  */
-	public function parse($argv, $command = null) {
-		if (isset($this->_subcommands[$command]) && $this->_subcommands[$command]->parser()) {
+	public function parse($argv) {
+		$command = isset($argv[0]) ? $argv[0] : null;
+		if (isset($this->_subcommands[$command])) {
 			array_shift($argv);
+		}
+		if (isset($this->_subcommands[$command]) && $this->_subcommands[$command]->parser()) {
 			return $this->_subcommands[$command]->parser()->parse($argv);
 		}
 		$params = $args = [];

src/Console/Shell.php

@@ -338,11 +338,11 @@ class Shell {
  * @return void
  * @link http://book.cakephp.org/2.0/en/console-and-shells.html#Shell::runCommand
  */
-	public function runCommand($command, $argv) {
-
+	public function runCommand($argv) {
+		$command = isset($argv[0]) ? $argv[0] : null;
 		$this->OptionParser = $this->getOptionParser();
 		try {
-			list($this->params, $this->args) = $this->OptionParser->parse($argv, $command);
+			list($this->params, $this->args) = $this->OptionParser->parse($argv);
 		} catch (Error\ConsoleException $e) {
 			$this->err('<error>Error: ' . $e->getMessage() . '</error>');
 			$this->out($this->OptionParser->help($command));
@@ -367,21 +367,22 @@ class Shell {
 		$subcommands = $this->OptionParser->subcommands();
 		$isMethod = $this->hasMethod($command);
 
-		if (
-			$isMethod &&
-			(count($subcommands) === 0 || isset($subcommands[$command]))
-		) {
+		if ($isMethod && count($subcommands) === 0) {
 			array_shift($this->args);
 			$this->startup();
 			return call_user_func_array([$this, $command], $this->args);
 		}
 
+		if ($isMethod && isset($subcommands[$command])) {
+			$this->startup();
+			return call_user_func_array([$this, $command], $this->args);
+		}
+
 		if ($this->hasTask($command) && isset($subcommands[$command])) {
-			array_shift($argv);
 			$this->startup();
 			$command = Inflector::camelize($command);
-			// TODO this is suspicious.
-			return $this->{$command}->runCommand('execute', $argv);
+			$argv[0] = 'execute';
+			return $this->{$command}->runCommand($argv);
 		}
 
 		if ($this->hasMethod('main')) {

src/Console/ShellDispatcher.php

@@ -159,13 +159,8 @@ class ShellDispatcher {
 
 		$Shell = $this->findShell($shell);
 
-		$command = null;
-		if (isset($this->args[0])) {
-			$command = $this->args[0];
-		}
-
 		$Shell->initialize();
-		return $Shell->runCommand($command, $this->args);
+		return $Shell->runCommand($this->args);
 	}
 
 /**

tests/TestCase/Console/Command/CompletionShellTest.php

@@ -83,7 +83,7 @@ class CompletionShellTest extends TestCase {
  * @return void
  */
 	public function testStartup() {
-		$this->Shell->runCommand('main', array());
+		$this->Shell->runCommand(['main']);
 		$output = $this->out->output;
 
 		$needle = 'Welcome to CakePHP';
@@ -96,7 +96,7 @@ class CompletionShellTest extends TestCase {
  * @return void
  */
 	public function testMain() {
-		$this->Shell->runCommand('main', array());
+		$this->Shell->runCommand(['main']);
 		$output = $this->out->output;
 
 		$expected = "/This command is not intended to be called manually/";
@@ -109,7 +109,7 @@ class CompletionShellTest extends TestCase {
  * @return void
  */
 	public function testCommands() {
-		$this->Shell->runCommand('commands', array());
+		$this->Shell->runCommand(['commands']);
 		$output = $this->out->output;
 
 		$expected = "TestPlugin.example TestPluginTwo.example TestPluginTwo.welcome bake i18n server test sample\n";
@@ -122,7 +122,7 @@ class CompletionShellTest extends TestCase {
  * @return void
  */
 	public function testOptionsNoArguments() {
-		$this->Shell->runCommand('options', array());
+		$this->Shell->runCommand(['options']);
 		$output = $this->out->output;
 
 		$expected = "--help -h --verbose -v --quiet -q\n";
@@ -135,7 +135,7 @@ class CompletionShellTest extends TestCase {
  * @return void
  */
 	public function testOptionsNonExistingCommand() {
-		$this->Shell->runCommand('options', array('options', 'foo'));
+		$this->Shell->runCommand(['options', 'foo']);
 		$output = $this->out->output;
 
 		$expected = "--help -h --verbose -v --quiet -q\n";
@@ -148,7 +148,7 @@ class CompletionShellTest extends TestCase {
  * @return void
  */
 	public function testOptions() {
-		$this->Shell->runCommand('options', array('options', 'bake'));
+		$this->Shell->runCommand(['options', 'bake']);
 		$output = $this->out->output;
 
 		$expected = "--help -h --verbose -v --quiet -q --connection -c --theme -t\n";
@@ -161,7 +161,7 @@ class CompletionShellTest extends TestCase {
  * @return void
  */
 	public function testSubCommandsCorePlugin() {
-		$this->Shell->runCommand('subcommands', array('subcommands', 'CORE.bake'));
+		$this->Shell->runCommand(['subcommands', 'CORE.bake']);
 		$output = $this->out->output;
 
 		$expected = "behavior component controller fixture helper model plugin project shell test view widget zerg\n";
@@ -174,7 +174,7 @@ class CompletionShellTest extends TestCase {
  * @return void
  */
 	public function testSubCommandsAppPlugin() {
-		$this->Shell->runCommand('subcommands', array('subcommands', 'app.sample'));
+		$this->Shell->runCommand(['subcommands', 'app.sample']);
 		$output = $this->out->output;
 
 		$expected = '';
@@ -187,7 +187,7 @@ class CompletionShellTest extends TestCase {
  * @return void
  */
 	public function testSubCommandsPlugin() {
-		$this->Shell->runCommand('subcommands', array('subcommands', 'TestPluginTwo.welcome'));
+		$this->Shell->runCommand(['subcommands', 'TestPluginTwo.welcome']);
 		$output = $this->out->output;
 
 		$expected = "say_hello\n";
@@ -200,7 +200,7 @@ class CompletionShellTest extends TestCase {
  * @return void
  */
 	public function testSubCommandsNoArguments() {
-		$this->Shell->runCommand('subcommands', array());
+		$this->Shell->runCommand(['subcommands']);
 		$output = $this->out->output;
 
 		$expected = '';
@@ -213,7 +213,7 @@ class CompletionShellTest extends TestCase {
  * @return void
  */
 	public function testSubCommandsNonExistingCommand() {
-		$this->Shell->runCommand('subcommands', array('subcommands', 'foo'));
+		$this->Shell->runCommand(['subcommands', 'foo']);
 		$output = $this->out->output;
 
 		$expected = '';
@@ -226,7 +226,7 @@ class CompletionShellTest extends TestCase {
  * @return void
  */
 	public function testSubCommands() {
-		$this->Shell->runCommand('subcommands', array('subcommands', 'bake'));
+		$this->Shell->runCommand(['subcommands', 'bake']);
 		$output = $this->out->output;
 
 		$expected = "behavior component controller fixture helper model plugin project shell test view widget zerg\n";
@@ -239,7 +239,7 @@ class CompletionShellTest extends TestCase {
  * @return void
  */
 	public function testFuzzy() {
-		$this->Shell->runCommand('fuzzy', array());
+		$this->Shell->runCommand(['fuzzy']);
 		$output = $this->out->output;
 
 		$expected = '';

tests/TestCase/Console/ConsoleOptionParserTest.php

@@ -633,7 +633,7 @@ TEXT;
 				)
 			));
 
-		$result = $parser->parse(array('--secondary', '--fourth', '4', 'c'), 'sub');
+		$result = $parser->parse(array('sub', '--secondary', '--fourth', '4', 'c'));
 		$expected = array(array(
 			'secondary' => true,
 			'fourth' => '4',

tests/TestCase/Console/ShellDispatcherTest.php

@@ -130,7 +130,7 @@ class ShellDispatcherTest extends TestCase {
 
 		$Shell->expects($this->once())->method('initialize');
 		$Shell->expects($this->once())->method('runCommand')
-			->with(null, array())
+			->with([])
 			->will($this->returnValue(true));
 
 		$dispatcher->expects($this->any())
@@ -155,7 +155,7 @@ class ShellDispatcherTest extends TestCase {
 
 		$Shell->expects($this->once())->method('initialize');
 		$Shell->expects($this->once())->method('runCommand')
-			->with('initdb', array('initdb'))
+			->with(['initdb'])
 			->will($this->returnValue(true));
 
 		$dispatcher->expects($this->any())

tests/TestCase/Console/ShellTest.php

@@ -531,7 +531,7 @@ class ShellTest extends TestCase {
 		$shell->expects($this->once())->method('main')
 			->with('cakes')
 			->will($this->returnValue(true));
-		$result = $shell->runCommand('cakes', ['cakes', '--verbose']);
+		$result = $shell->runCommand(['cakes', '--verbose']);
 		$this->assertTrue($result);
 	}
 
@@ -548,7 +548,7 @@ class ShellTest extends TestCase {
 		$shell->expects($this->once())->method('hit_me')
 			->with('cakes')
 			->will($this->returnValue(true));
-		$result = $shell->runCommand('hit_me', ['hit_me', 'cakes', '--verbose']);
+		$result = $shell->runCommand(['hit_me', 'cakes', '--verbose']);
 		$this->assertTrue($result);
 	}
 
@@ -574,7 +574,7 @@ class ShellTest extends TestCase {
 		$shell->expects($this->never())->method('startup');
 		$shell->expects($this->never())->method('roll');
 
-		$result = $shell->runCommand('roll', ['roll', 'cakes', '--verbose']);
+		$result = $shell->runCommand(['roll', 'cakes', '--verbose']);
 		$this->assertFalse($result);
 	}
 
@@ -599,7 +599,7 @@ class ShellTest extends TestCase {
 			->method('slice')
 			->with('cakes');
 
-		$shell->runCommand('slice', ['slice', 'cakes', '--verbose']);
+		$shell->runCommand(['slice', 'cakes', '--verbose']);
 	}
 
 /**
@@ -623,7 +623,7 @@ class ShellTest extends TestCase {
 		$parser->expects($this->once())
 			->method('help');
 
-		$shell->runCommand('slice', ['slice', 'cakes', '--verbose']);
+		$shell->runCommand(['slice', 'cakes', '--verbose']);
 	}
 
 /**
@@ -641,7 +641,7 @@ class ShellTest extends TestCase {
 		$shell->expects($this->never())->method('hr');
 		$shell->expects($this->once())->method('out');
 
-		$shell->runCommand('hr', array());
+		$shell->runCommand(['hr']);
 	}
 
 /**
@@ -658,7 +658,7 @@ class ShellTest extends TestCase {
 			->will($this->returnValue($parser));
 		$shell->expects($this->once())->method('out');
 
-		$result = $shell->runCommand('idontexist', array());
+		$result = $shell->runCommand(['idontexist']);
 		$this->assertFalse($result);
 	}
 
@@ -679,7 +679,7 @@ class ShellTest extends TestCase {
 			->will($this->returnValue($Parser));
 		$Shell->expects($this->once())->method('out');
 
-		$Shell->runCommand(null, array('--help'));
+		$Shell->runCommand(['--help']);
 	}
 
 /**
@@ -701,7 +701,7 @@ class ShellTest extends TestCase {
 		$shell->expects($this->once())->method('out');
 		$shell->RunCommand = $task;
 
-		$result = $shell->runCommand('run_command', ['run_command', 'one']);
+		$result = $shell->runCommand(['run_command', 'one']);
 		$this->assertFalse($result);
 	}
 
@@ -718,7 +718,7 @@ class ShellTest extends TestCase {
 		$task = $this->getMock('Cake\Console\Shell', ['execute', 'runCommand'], [], '', false);
 		$task->expects($this->any())
 			->method('runCommand')
-			->with('execute', ['one']);
+			->with(['execute', 'one']);
 
 		$shell->expects($this->once())->method('getOptionParser')
 			->will($this->returnValue($parser));
@@ -729,7 +729,7 @@ class ShellTest extends TestCase {
 			->will($this->returnValue(true));
 
 		$shell->Slice = $task;
-		$shell->runCommand('slice', ['slice', 'one']);
+		$shell->runCommand(['slice', 'one']);
 	}
 
 /**
@@ -799,7 +799,7 @@ TEXT;
 			->with(false);
 
 		$this->Shell = $this->getMock(__NAMESPACE__ . '\ShellTestShell', array('_useLogger'), array($io));
-		$this->Shell->runCommand('foo', array('--quiet'));
+		$this->Shell->runCommand(['foo', '--quiet']);
 	}
 
 }