merged branch Seldaek/consmerge (PR #7650)

This PR was merged into the 2.2 branch.

Discussion
----------

[Console] Fix merging of application definition

Fixes #7068, replaces #7158 - the fix there introduces a regression and always shows the application arguments in the commands help (i.e. it lists "command" as an argument to every command, except list because it overrides getNativeDefinition).

Commits
-------

46909fa [Console] Fix merging of application definition, fixes #7068, replaces #7158
This commit is contained in:
Fabien Potencier 2013-04-12 15:21:28 +02:00
commit 96251acbae
2 changed files with 29 additions and 1 deletions

View File

@ -36,6 +36,7 @@ class Command
private $description; private $description;
private $ignoreValidationErrors; private $ignoreValidationErrors;
private $applicationDefinitionMerged; private $applicationDefinitionMerged;
private $applicationDefinitionMergedWithArgs;
private $code; private $code;
private $synopsis; private $synopsis;
private $helperSet; private $helperSet;
@ -54,6 +55,7 @@ class Command
$this->definition = new InputDefinition(); $this->definition = new InputDefinition();
$this->ignoreValidationErrors = false; $this->ignoreValidationErrors = false;
$this->applicationDefinitionMerged = false; $this->applicationDefinitionMerged = false;
$this->applicationDefinitionMergedWithArgs = false;
$this->aliases = array(); $this->aliases = array();
if (null !== $name) { if (null !== $name) {
@ -277,7 +279,7 @@ class Command
*/ */
private function mergeApplicationDefinition($mergeArgs = true) private function mergeApplicationDefinition($mergeArgs = true)
{ {
if (null === $this->application || true === $this->applicationDefinitionMerged) { if (null === $this->application || (true === $this->applicationDefinitionMerged && ($this->applicationDefinitionMergedWithArgs || !$mergeArgs))) {
return; return;
} }
@ -290,6 +292,9 @@ class Command
$this->definition->addOptions($this->application->getDefinition()->getOptions()); $this->definition->addOptions($this->application->getDefinition()->getOptions());
$this->applicationDefinitionMerged = true; $this->applicationDefinitionMerged = true;
if ($mergeArgs) {
$this->applicationDefinitionMergedWithArgs = true;
}
} }
/** /**

View File

@ -193,6 +193,29 @@ class CommandTest extends \PHPUnit_Framework_TestCase
$this->assertEquals(3, $command->getDefinition()->getArgumentCount(), '->mergeApplicationDefinition() does not try to merge twice the application arguments and options'); $this->assertEquals(3, $command->getDefinition()->getArgumentCount(), '->mergeApplicationDefinition() does not try to merge twice the application arguments and options');
} }
public function testMergeApplicationDefinitionWithoutArgsThenWithArgsAddsArgs()
{
$application1 = new Application();
$application1->getDefinition()->addArguments(array(new InputArgument('foo')));
$application1->getDefinition()->addOptions(array(new InputOption('bar')));
$command = new \TestCommand();
$command->setApplication($application1);
$command->setDefinition($definition = new InputDefinition(array()));
$r = new \ReflectionObject($command);
$m = $r->getMethod('mergeApplicationDefinition');
$m->setAccessible(true);
$m->invoke($command, false);
$this->assertTrue($command->getDefinition()->hasOption('bar'), '->mergeApplicationDefinition(false) merges the application and the commmand options');
$this->assertFalse($command->getDefinition()->hasArgument('foo'), '->mergeApplicationDefinition(false) does not merge the application arguments');
$m->invoke($command, true);
$this->assertTrue($command->getDefinition()->hasArgument('foo'), '->mergeApplicationDefinition(true) merges the application arguments and the command arguments');
$m->invoke($command);
$this->assertEquals(2, $command->getDefinition()->getArgumentCount(), '->mergeApplicationDefinition() does not try to merge twice the application arguments');
}
public function testRun() public function testRun()
{ {
$command = new \TestCommand(); $command = new \TestCommand();