Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[2.2][Console] default command #3857

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 1 commit into from
Closed

[2.2][Console] default command #3857

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
35 changes: 34 additions & 1 deletion 35 src/Symfony/Component/Console/Application.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class Application
private $autoExit;
private $definition;
private $helperSet;

private $defaultCommandName = null;
/**
Copy link
Member

Choose a reason for hiding this comment

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

missing empty line between the properties and the ocnstructor

* Constructor.
*
Expand All @@ -80,6 +80,30 @@ public function __construct($name = 'UNKNOWN', $version = 'UNKNOWN')
}
}

/**
* Sets the name of the default command, this command will be executed if
* no command is provided in the CLI arguments
* @param String $command
Copy link
Member

Choose a reason for hiding this comment

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

string is lowercases in the phpdoc.

And your phpdoc does not follow the standard formatting: you should have a short description (Sets the name of the default command. in your case), then an empty line before the long description, and another empty line before the param tags (well, empty except the *)

Copy link
Author

Choose a reason for hiding this comment

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

I can change it, but it follows the examples you can find on phpdoc site

Copy link
Member

Choose a reason for hiding this comment

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

the phpdoc site really uses an uppercased name for built-in types ? This will break autocompletion in some IDEs (which will consider it as a class name)

Btw, I don't see such format in the doc of phpdocumentor on phpdoc.org or in the doc of PHPDoc on phpdoc.de (from the PHP4 times). Which site are you talking about ?

Copy link
Author

Choose a reason for hiding this comment

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

I followed the examples on http://manual.phpdoc.org/ and there is no need to separate the short description from the tags with two newlines, and it is not very clear about the case of types

but this is not really a problem

Copy link
Member

Choose a reason for hiding this comment

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

separating the tags from the description is not mandatory when using phpdoc. But it is what is done in the Symfony source code

*/
public function setDefaultCommandName($command)
{
if (!is_string($command)) {
throw new \UnexpectedValueException('Default Command Name must be a String');
}
$this->defaultCommandName = $command;
}

/**
* gets the name of the default command, this command will be executed if
* no command is provided in the CLI arguments

* @return String
*/
public function getDefaultCommandName()
{
return $this->defaultCommandName ;
}

/**
* Runs the current application.
*
Expand All @@ -96,6 +120,15 @@ public function run(InputInterface $input = null, OutputInterface $output = null
{
if (null === $input) {
$input = new ArgvInput();
// if we have a default command and we can not decide which command
// to execute we execute the default one
if (! is_null($this->getDefaultcommandName())) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be null !== $this->getDefaultcommandName()

Copy link
Author

Choose a reason for hiding this comment

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

any specific reason on why it should be that way ?

Copy link
Contributor

Choose a reason for hiding this comment

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

http://symfony.com/doc/current/contributing/code/standards.html

Copy link
Contributor

Choose a reason for hiding this comment

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

That doesn’t answer the question.

Copy link
Member

Choose a reason for hiding this comment

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

it does: this is one of our coding standard rules.

Copy link
Member

Choose a reason for hiding this comment

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

arf, I hate incomplete docs

Copy link
Contributor

Choose a reason for hiding this comment

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

I also don't know answer to question “is there something wrong with is_null that requires the use of !== instead?”, but I have found:

  1. http://trac.symfony-project.org/wiki/HowToContributeToSymfony#CodingStandards
  2. Remove all is_null alias calls. #1202

Copy link
Contributor

Choose a reason for hiding this comment

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

I’m guessing that is_null is an unnessary extra function call. Moreover, nobody wants to have to remember the exact behaviour in every possible situation of yet another, potentially dubiously-coded, way of testing for inexistence/invalidity/emptiness.

As for the order of the operands (the Yoda condition), I think it’s because some people have trouble remembering to put the correct number of equal signs when testing for $foo === null, and putting only one results in an assignment. So they reverse the operands (because this is somehow easier to remember?), so that putting only one equal sign is a syntax error (null = $foo). Then they do the same for every comparison operator, whatever the operands, even if you can’t assign, even accidentally, to $foo->getStuff(), for “consistency”.

Copy link
Member

Choose a reason for hiding this comment

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

@max-voloshin regarding is_null vs null ===, it is a matter of consistency in the codebase.

and looking at http://trac.symfony-project.org for Symfony2 is wrong. symfony-project.org is the symfony1 domain so all resources on it are not related to Symfony2. Symfony2 lives on symfony.com

Copy link
Contributor

Choose a reason for hiding this comment

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

@max-voloshin regarding is_null vs null ===, it is a matter of consistency in the codebase.

@stof, no problem. I encourage consistency in the codebase also

and looking at http://trac.symfony-project.org for Symfony2 is wrong. symfony-project.org is the symfony1 domain so all resources on it are not related to Symfony2. Symfony2 lives on symfony.com

I just have remembered where I have seen notes about this fact:)

if (! $this->has($this->getCommandName($input))) {
$argv = $_SERVER['argv'];
Copy link
Contributor

Choose a reason for hiding this comment

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

Isnt this wrong? Command could possibly be called from a webserver instead of the command line.

Copy link
Member

Choose a reason for hiding this comment

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

it is

array_splice($argv, 1, 0, $this->getDefaultcommandName());
$input = new ArgvInput($argv);
}
}
}

if (null === $output) {
Expand Down
37 changes: 37 additions & 0 deletions 37 src/Symfony/Component/Console/Tests/ApplicationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,27 @@ public function testRun()
$this->assertSame('called'.PHP_EOL, $tester->getDisplay(), '->run() does not call interact() if -n is passed');
}

/**
* Tests the run() with the default command set
*/
public function testRun_defaultCommand()
{
$application = new Application();
$application->setAutoExit(false);
$application->setCatchExceptions(false);
$application->add($command = new \Foo1Command());
$application->setDefaultCommandName('foo:bar1');
$_SERVER['argv'] = array('cli.php');

ob_start();
$application->run();
ob_end_clean();

$this->assertSame('Symfony\Component\Console\Input\ArgvInput', get_class($command->input), '->run() creates an ArgvInput by default if none is given');
$this->assertSame('Symfony\Component\Console\Output\ConsoleOutput', get_class($command->output), '->run() creates a ConsoleOutput by default if none is given');

}

/**
* @expectedException \LogicException
* @dataProvider getAddingAlreadySetDefinitionElementData
Expand Down Expand Up @@ -454,4 +475,20 @@ public function getAddingAlreadySetDefinitionElementData()
array(new InputOption('query', 'q', InputOption::VALUE_NONE)),
);
}

public function testSetGetDefaultCommandName()
{
$application = new Application();
$application->setDefaultCommandName('foo');
$this->assertEquals('foo', $application->getDefaultCommandName());
}

/**
* @expectedException \UnexpectedValueException
*/
public function testSetGetDefaultCommandName_InvalidParameter()
{
$application = new Application();
$application->setDefaultCommandName(new \stdClass);
}
}
Morty Proxy This is a proxified and sanitized view of the page, visit original site.