8000 [Console] command list ordering, styles in command names bugfix #12051 by spdionis · Pull Request #12500 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Console] command list ordering, styles in command names bugfix #12051 #12500

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

[Console] command list ordering, styles in command names bugfix #12051 #12500

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4f61339
global commands are always first in command list
Nov 18, 2014
1591f78
output command and namespace without styling
Nov 21, 2014
823407a
add test for command ordering and raw output
Mar 29, 2015
942c34e
make test less fragile
Mar 29, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/Symfony/Component/Console/Descriptor/ApplicationDescription.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,15 +134,17 @@ private function inspectApplication()
private function sortCommands(array $commands)
{
$namespacedCommands = array();
$globalCommands = array();
foreach ($commands as $name => $command) {
$key = $this->application->extractNamespace($name, 1);
if (!$key) {
$key = '_global';
$globalCommands['_global'][$name] = $command;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest the _global key here, build $globalCommands as being only the array of global commands themselves, and the prepending this array in the list of commands after the sorting with array_shift($namespacedCommands, $globalCommands) (and doing it only in case the list of global commands is not empty to avoid useless work)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, actually no. The switch from array_merge to array_shift won't work because the code expects to have a _global key for global commands. But the global key could be handle at the time of merging, keeping $globalCommands as the list of global commands (which is not the case currently)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you meant here. Anyway I know that $globalCommands['_global'] feels redunant but in my opinion is it fine. The array_merge call later looks pretty straightforward this way.

} else {
$namespacedCommands[$key][$name] = $command;
}

$namespacedCommands[$key][$name] = $command;
}
ksort($namespacedCommands);
$namespacedCommands = array_merge($globalCommands, $namespacedCommands);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add some unit tests to cover the cases explained in the ticket?

foreach ($namespacedCommands as &$commands) {
ksort($commands);
Expand Down
10 changes: 6 additions & 4 deletions src/Symfony/Component/Console/Descriptor/TextDescriptor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,13 +195,15 @@ protected function describeApplication(Application $application, array $options
// add commands by namespace
foreach ($description->getNamespaces() as $namespace) {
if (!$describedNamespace && ApplicationDescription::GLOBAL_NAMESPACE !== $namespace['id']) {
$this->writeText("\n");
$this->writeText('<comment>'.$namespace['id'].'</comment>', $options);
$this->writeText("\n<comment>", $options);
$this->writeText($namespace['id'], array_merge($options, array('raw_output' => true)));
$this->writeText("</comment>", $options);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also be tested

}

foreach ($namespace['commands'] as $name) {
$this->writeText("\n");
$this->writeText(sprintf(" <info>%-${width}s</info> %s", $name, $description->getCommand($name)->getDescription()), $options);
$this->writeText("\n <info>", $options);
$this->writeText(sprintf("%-${width}s", $name), array_merge($options, array('raw_output' => true)));
$this->writeText(sprintf("</info> %s", $description->getCommand($name)->getDescription()), $options);
}
}

Expand Down
13 changes: 13 additions & 0 deletions src/Symfony/Component/Console/Tests/Command/ListCommandTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,17 @@ public function testExecuteListsCommandsWithNamespaceArgument()

$this->assertEquals($output, $commandTester->getDisplay(true));
}

public function testExecuteListsCommandsNameAndNamespaceRaw()
{
require_once realpath(__DIR__.'/../Fixtures/Foo6Command.php');
$application = new Application();
$application->add(new \Foo6Command());
$commandTester = new CommandTester($command = $application->get('list'));
$commandTester->execute(array('command' => $command->getName()));

$regex = '/Available commands:\s*help\s*.*\s*list.*\s*<fg=blue>foo\s*<fg=blue>foo:bar<\/fg=blue>/';

$this->assertRegExp($regex, $commandTester->getDisplay(true));
}
}
13 changes: 13 additions & 0 deletions src/Symfony/Component/Console/Tests/Fixtures/Foo6Command.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?php


use Symfony\Component\Console\Command\Command;

class Foo6Command extends Command
{
protected function configure()
{
$this->setName('<fg=blue>foo:bar</fg=blue>');
}

}
0