Skip to content

Commit

Permalink
[Console] Fix merging of application definition, fixes #7068, replaces
Browse files Browse the repository at this point in the history
  • Loading branch information
Seldaek committed Apr 12, 2013
1 parent 3c44d48 commit 46909fa
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
7 changes: 6 additions & 1 deletion src/Symfony/Component/Console/Command/Command.php
Expand Up @@ -36,6 +36,7 @@ class Command
private $description;
private $ignoreValidationErrors;
private $applicationDefinitionMerged;
private $applicationDefinitionMergedWithArgs;
private $code;
private $synopsis;
private $helperSet;
Expand All @@ -54,6 +55,7 @@ public function __construct($name = null)
$this->definition = new InputDefinition();
$this->ignoreValidationErrors = false;
$this->applicationDefinitionMerged = false;
$this->applicationDefinitionMergedWithArgs = false;
$this->aliases = array();

if (null !== $name) {
Expand Down Expand Up @@ -277,7 +279,7 @@ public function setCode($code)
*/
private function mergeApplicationDefinition($mergeArgs = true)
{
if (null === $this->application || true === $this->applicationDefinitionMerged) {
if (null === $this->application || (true === $this->applicationDefinitionMerged && ($this->applicationDefinitionMergedWithArgs || !$mergeArgs))) {
return;
}

Expand All @@ -290,6 +292,9 @@ private function mergeApplicationDefinition($mergeArgs = true)
$this->definition->addOptions($this->application->getDefinition()->getOptions());

$this->applicationDefinitionMerged = true;
if ($mergeArgs) {
$this->applicationDefinitionMergedWithArgs = true;
}
}

/**
Expand Down
23 changes: 23 additions & 0 deletions src/Symfony/Component/Console/Tests/Command/CommandTest.php
Expand Up @@ -193,6 +193,29 @@ public function testMergeApplicationDefinition()
$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()
{
$command = new \TestCommand();
Expand Down

0 comments on commit 46909fa

Please sign in to comment.