Navigation Menu

Skip to content

Commit

Permalink
Shift subcommand propagation to be at help generation.
Browse files Browse the repository at this point in the history
The old approach created a parser for every command that didn't have an
explict option parser. This broke automatic option inheritance for
subcommands. While there weren't any tests for this behavior many
console tools rely on this behavior. eg. `orm_cache`
  • Loading branch information
markstory committed Jan 16, 2017
1 parent 40362ba commit df36f91
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 37 deletions.
14 changes: 12 additions & 2 deletions src/Console/ConsoleInputSubcommand.php
Expand Up @@ -32,14 +32,14 @@ class ConsoleInputSubcommand
*
* @var string
*/
protected $_name;
protected $_name = '';

/**
* Help string for the subcommand
*
* @var string
*/
protected $_help;
protected $_help = '';

/**
* The ConsoleOptionParser for this subcommand.
Expand Down Expand Up @@ -83,6 +83,16 @@ public function name()
return $this->_name;
}

/**
* Get the raw help string for this command
*
* @return string
*/
public function getRawHelp()
{
return $this->_help;
}

/**
* Generate the help for this this subcommand.
*
Expand Down
45 changes: 10 additions & 35 deletions src/Console/ConsoleOptionParser.php
Expand Up @@ -583,8 +583,6 @@ public function addSubcommand($name, array $options = [])
];
$options += $defaults;

$options = $this->_mergeSubcommandHelpToParserDescription($options);

$command = new ConsoleInputSubcommand($options);
}
$this->_subcommands[$name] = $command;
Expand All @@ -593,33 +591,6 @@ public function addSubcommand($name, array $options = [])
return $this;
}

/**
* @param array $options Options
* @return array
*/
protected function _mergeSubcommandHelpToParserDescription($options)
{
if (!$options['help']) {
return $options;
}

if ($options['parser'] instanceof self) {
if ($options['parser']->getDescription() !== null) {
return $options;
}

$options['parser']->setDescription($options['help']);

return $options;
}

$options['parser'] = [
'description' => $options['help']
] + (array)$options['parser'];

return $options;
}

/**
* Remove a subcommand from the option parser.
*
Expand Down Expand Up @@ -750,12 +721,16 @@ public function parse($argv)
*/
public function help($subcommand = null, $format = 'text', $width = 72)
{
if (isset($this->_subcommands[$subcommand]) &&
$this->_subcommands[$subcommand]->parser() instanceof self
) {
$subparser = $this->_subcommands[$subcommand]->parser();
$subparser->setCommand($this->getCommand() . ' ' . $subparser->getCommand());

if (isset($this->_subcommands[$subcommand])) {
$command = $this->_subcommands[$subcommand];
$subparser = $command->parser();
if (!($subparser instanceof self)) {
$subparser = clone $this;
}
if (strlen($subparser->getDescription()) === 0) {
$subparser->setDescription($command->getRawHelp());
}
$subparser->setCommand($this->getCommand() . ' ' . $subcommand);
return $subparser->help(null, $format, $width);
}

Expand Down
24 changes: 24 additions & 0 deletions tests/TestCase/Console/ConsoleOptionParserTest.php
Expand Up @@ -591,6 +591,30 @@ public function testSubcommand()
$this->assertEquals($parser, $result, 'Adding a subcommand is not chainable');
}

/**
* Test addSubcommand inherits options as no
* parser is created.
*
* @return void
*/
public function testAddSubcommandInheritOptions()
{
$parser = new ConsoleOptionParser('test', false);
$parser->addSubcommand('build', [
'help' => 'Build things'
])->addOption('connection', [
'short' => 'c',
'default' => 'default'
])->addArgument('name', ['required' => false]);

$result = $parser->parse(['build']);
$this->assertEquals('default', $result[0]['connection']);

$result = $parser->subcommands();
$this->assertArrayHasKey('build', $result);
$this->assertFalse($result['build']->parser(), 'No parser should be created');
}

/**
* test addSubcommand with an object.
*
Expand Down

0 comments on commit df36f91

Please sign in to comment.