8000 [Console] Allow a command iterator to be added to console app by andytson · Pull Request #24990 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Console] Allow a command iterator to be added to console app #24990

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

[Console] Allow a command iterator to be added to console app #24990

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
[Console] Allow a command iterator to be added to console app 
Symfony 3.4/4.0 brings in the ability to use !tagged in service definitions to pass tagged services as a argument. It uses a RewindableGenerator class, so the typehint here doesn't accept it even though it just iterates over the commands.

This change will allow a custom console app to be created with a service method call in the definition.

The typehint is only available in PHP 7.1+ though, so isn't compatible with Symfony 3.4's targeted PHP versions, but is for 4.0.
  • Loading branch information
@andytson
andytson authored Nov 16, 2017
commit bba7e9d70a166889fbf618779b0eb66f7c694b96
4 changes: 2 additions & 2 deletions src/Symfony/Component/Console/Application.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,9 +390,9 @@ public function register($name)
*
* If a Command is not enabled it will not be added.
*
* @param Command[] $commands An array of commands
* @param Command[] $commands An array or iterator of commands
*/
public function addCommands(array $commands)
public function addCommands(iterable $commands)
Copy link
Contributor

Choose a reason for hiding this comment

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

That would be a BC break unfortunately, as it's a public method, so we can't change this prototype before 5.0.

Copy link
Author

Choose a reason for hiding this comment

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

it's not a BC break when PHP 7.1 is required in Symfony 4, which is still beta. It's a super class/primative type hint, which accepts arrays as well as iterators. It only allows more than before, however though in the event this method is overriden in a sub-class, then it'd not satisfy the function signature, if that counts.

Copy link
Member

Choose a reason for hiding this comment

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

Application API expects $commands to be an array elsewhere, passing an iterable here would break at e.g. find(). So we would need to iterator_to_array(). Is there a real benefit? I don't see one personally.

Copy link
Member
@chalasr chalasr Nov 16, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Oh, we iterate over the result just below, nevermind.

{
foreach ($commands as $command) {
$this->add($command);
Expand Down
0