merged branch bamarni/console (PR #5689)

This PR was merged into the 2.1 branch.

Commits
-------

86503db [Console] added a unit test
3b2eeb6 [Console] fixed #5384

Discussion
----------

[Console] Simplified find method

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #5384

It simplifies the way command or command suggestions are found, also fixing issue #5384.

It's a WIP I had, my plan was to also remove the original methods findAlternatives, etc... but some other methods are relying on them, so I'm sending it as it instead of running into more refactoring.

---------------------------------------------------------------------------

by bamarni at 2012-10-06T20:35:59Z

I've refactored some code, as you can see in the tests I've changed, the exception messages have slightly changed.

---------------------------------------------------------------------------

by fabpot at 2012-10-08T13:04:36Z

I don't see the point fo this big refactoring, especially if it also changes the behavior. I think a better approach would be to do only one thing at a time. But as is, this is not mergeable.

---------------------------------------------------------------------------

by bamarni at 2012-10-08T14:01:41Z

I'll try to fix this PR to keep the same behavior and error messages as it is currently.

---------------------------------------------------------------------------

by bamarni at 2012-10-08T14:07:43Z

Well if in fact there is only one bug, it would be better to patch the current code instead of doing this refactoring, even though I think the current code which finds commands is overcomplicated.

---------------------------------------------------------------------------

by stof at 2012-10-08T14:09:18Z

Well, in this case, please submit only a bug report for the 2.1 branch (or 2.0 if it is also affected), and then you can refactor the logic in master to simplify it if needed. But a refactoring should not happen in a maintenance branch

---------------------------------------------------------------------------

by bamarni at 2012-10-09T07:32:47Z

I've patched the code.

---------------------------------------------------------------------------

by fabpot at 2012-10-09T07:37:53Z

Can you add a test for the bug fix?

---------------------------------------------------------------------------

by bamarni at 2012-10-09T21:30:21Z

here you go

src/Symfony/Component/Console/Application.php

@@ -545,7 +545,10 @@ class Application
         // name
         $commands = array();
         foreach ($this->commands as $command) {
-            if ($this->extractNamespace($command->getName()) == $namespace) {
+            $extractedNamespace = $this->extractNamespace($command->getName());
+            if ($extractedNamespace === $namespace
+               || !empty($namespace) && 0 === strpos($extractedNamespace, $namespace)
+            ) {
                 $commands[] = $command->getName();
             }
         }
@@ -565,7 +568,10 @@ class Application
         $aliases = array();
         foreach ($this->commands as $command) {
             foreach ($command->getAliases() as $alias) {
-                if ($this->extractNamespace($alias) == $namespace) {
+                $extractedNamespace = $this->extractNamespace($alias);
+                if ($extractedNamespace === $namespace
+                   || !empty($namespace) && 0 === strpos($extractedNamespace, $namespace)
+                ) {
                     $aliases[] = $alias;
                 }
             }

src/Symfony/Component/Console/Tests/ApplicationTest.php

@@ -30,6 +30,7 @@ class ApplicationTest extends \PHPUnit_Framework_TestCase
         require_once self::$fixturesPath.'/Foo1Command.php';
         require_once self::$fixturesPath.'/Foo2Command.php';
         require_once self::$fixturesPath.'/Foo3Command.php';
+        require_once self::$fixturesPath.'/Foo4Command.php';
     }
 
     protected function normalizeLineBreaks($text)
@@ -259,6 +260,19 @@ class ApplicationTest extends \PHPUnit_Framework_TestCase
             $this->assertInstanceOf('\InvalidArgumentException', $e, '->find() throws an \InvalidArgumentException if command does not exist, with alternatives');
             $this->assertRegExp('/Did you mean one of these/', $e->getMessage(), '->find() throws an \InvalidArgumentException if command does not exist, with alternatives');
         }
+
+        $application->add(new \Foo3Command());
+        $application->add(new \Foo4Command());
+
+        // Subnamespace + plural
+        try {
+            $a = $application->find('foo3:');
+            $this->fail('->find() should throw an \InvalidArgumentException if a command is ambiguous because of a subnamespace, with alternatives');
+        } catch (\Exception $e) {
+            $this->assertInstanceOf('\InvalidArgumentException', $e);
+            $this->assertRegExp('/foo3:bar/', $e->getMessage());
+            $this->assertRegExp('/foo3:bar:toh/', $e->getMessage());
+        }
     }
 
     public function testFindAlternativeCommands()

src/Symfony/Component/Console/Tests/Fixtures/Foo4Command.php

@@ -0,0 +1,11 @@
+<?php
+
+use Symfony\Component\Console\Command\Command;
+
+class Foo4Command extends Command
+{
+    protected function configure()
+    {
+        $this->setName('foo3:bar:toh');
+    }
+}