[RFC][3.0] Enhancing the Console component

[wouterj]

In 3.0, we want to use DI better for the Console component. The main problem for this is that the Command class handles the definition of the command as well as the execution. This should be splitted.

When thinking about this, I got a new idea which means that we should completely refactor the Console component. We should make it more consistent with the HttpFoundation story.

When executing a command, it is a CliRequest (ConsoleRequest, ...). This should be handled by a ConsoleRouter, which loads the console routing files with the definitions of the commands. The command will be nothing more than a normal controller, except that it retrieves a CliRequest and returns a CliResponse.

This will get some issues with interacting with the console. But as long as the controller uses helpers to do that, I don't think it will be much of a problem. Honestly, this idea isn't 100% good right now and it needs a lot of discussion and tweaking to get it correct, but I just wanted to get some responses to this idea :)

[cordoval]

we should also check the work of @pmjones on aura2.0 cli even though it misses the application part so we can decouple things. Zf2 has a component that is similar as well. I agree with your ideas @wouterj. I think the locator and dependency injection is a balance, we cannot go everything DI, there should be room for each pattern in its right measure.

[stof]

Returning a CliResponse will not work well. While it is fine for the exit code (equivalent to the HTTP status code), it is not for the output. In a CLI environment, we want to stream the output, not wait until the end to display it.

[fabpot]

I very disagree with this approach. In fact, I disagree with everything. I don't want to use the DIC for the console component. I really like the fact that you can very easily create a command without any other deps and no plumbing at all.

Keep in mind that interacting with the CLI is very different from interacting with an HTTP request; that's why we have 2 different components with 2 different approaches in Symfony.

[stof]

@fabpot the place where the usage of DI could be improved is accessing the Output in services/helpers. The Input and Output are actually quite different in the Console component. The Input is a value object (equivalent of the Request object in HttpFoundation) while the output is a service handling the display. Currently, it is the equivalent of a scoped service because of the way it is passed.
If we were cleaning this inconsistency, the isInteractive flag should probably move to a different place than the Input too (it is not really part of the Input, which does not handle the interactive input but only the initial arguments).

so while I think some changes could be done to improve the Console component, I also disagree with pretty much everything suggested by @wouterj here.

[stof]

Another point that we could improve though is the lazy-loading of commands when using them as services, to avoid building all their dependencies. This could be done by requiring to pass the command name as an attribute of the tag to be able to retrieve the command instance only when it needs to be called

[cordoval]

@stof what is the reason after doing CAS for commands we still cannot just tag them lazy : true? so the proxymanager thing only works for a particular kind of service or because of the DI? @wouterj has a half point, i also disagree, but i agree that more needs to be done to decouple the application part from the low level parts of the console component imo.

[Ocramius]

This issue is the only thing that is actually keeping me from not proposing kicking out zendframework/zend-console for symfony/console (I'm currently mashing up both to make a Symfony\Component\Console\Application behave as @wouterj described it) :-)

Having the console work homogeneously with the rest of the framework reduces the amount of documentation, knowledge required for working with it and would basically make commands controllers or services, which is (in my opinion) plain awesome.

This also would finally get something more generic than Symfony\Component\HttpFoundation request/response and kernel, throwing SF3 out of the "http-only" context, which is quite a limitation in my opinion.

I'm not sure how much of a BC break there is here though, so it may be a complete no-go depending on what kind of compatibility 3.x wants to provide for 2.x, because I see this issue as a huge rewrite of many components :\

[stof]

@cordoval The Application needs the console name. So even if you tag the service as lazy, it will be useless because getName will initialize the proxy

@Ocramius IMO, trying to use the same Request/Response flow for the Console is a big mistake. The CLI is interactive, while HTTP is not. A CLI is not a simple request/response

[Ocramius]

@stof if the request and the response encapsulate a stream (actually, the current I/O objects could be wrapped in a request/response as well)

[jakzal]

👎 on trying to bring the http dictionary to the command line. Decoupling definitions from execution (if needed at all) doesn't need to go into that direction.

[realityking]

If the main goal is to have commands as a service without having to initialize all dependencies, couldn't the definition of the command not be moved to a static method?

[gnugat]

I've discussed something like this 6 month ago: https://gist.github.com/gnugat/6899483
The conclusions so far:

• HTTP and CLI are different:
  • we shouldn't share the same vocabulary (like @jakzal said)
  • Response is a value object, Output a service which writes in a stream
  • the command returns only an exit status (no need for a value object)
• the Console component sure needs some love:
  • separating the responsibility of finding the right command could be delegated to a router
  • separating the command configuration from its actual action

Some novelties have made their way, we should keep them in mind:

• ConsoleLogger: messaging is done by services as log entries
• Helpers are being replaced (Table and ProgressBa)

About the interactivity: yes, you can stream responses in a web context. In a console context, it's not always used and when it is, it's mainly to spare the user from writing the options (just type the command name, and wait for the console to ask you the values for each options).

Finally, I'd like to bring your attention to the following use case: sometimes, the default values of command's options need to be retrieved using services (for example by making a database query). Currently doing so can impact performances...

[stof]

If the main goal is to have commands as a service without having to initialize all dependencies, couldn't the definition of the command not be moved to a static method?

not needed if we can improve the registration of such commands. The only thing needed when we are not running the command is its name, not the full config

[wouterj]

The only thing needed when we are not running the command is its name, not the full config

What about the help command? Or the --help option? They don't need to load all commands with all dependencies too, but they need to full config.

I don't want to use the DIC for the console component.

You don't have to, you can also use it without the container. However, when you want to use the container there should be good options for it. Currently, the command tag isn't a good solution.

After all your comments (thanks for them!), I can see why we shouldn't put a console in a HTTP thing using Requests and Responses. However, the way it works can be used for Stream operations too (in an Input and Output way).

I really like the example @gnugat gave months ago (and I'm sorry I've missed that one then, @gnugat). I don't see why we should differ that much between a Request/Response and I/O workflow.

[stof]

What about the help command? Or the --help option? They don't need to load all commands with all dependencies too, but they need to full config.

the help command will need to load a single command. In this case, loading the dependencies of this command is not a big issue IMO.

Btw, I think the registration of commands as services could be improved in the 2.x serie already if we require to pass the command name as an attribute on the tag

And regarding the suggestion of @gnugat, it does not make sense. The Output class is not a value object representing the output. It is a service sending things to the output. The command should not be responsible to create it at all. It is a dependency of the application, not a returned value

[gnugat]

@stof loading the command's dependencies when using help should still be possible: in some cases you want to retrieve information from services to use it as option default value.

Here's an example: I have a command which punches new time cards. I don't want my user to provide the start hour: based on the last time card I'm fully able to guess it: https://github.com/gnugat/tempo-simple/blob/5dd4f9167b6c79e8939a43f1faeb76aa8d188a0f/src/TempoSimple/Bundle/SpaghettiBundle/Command/PunchTimeCardCommand.php#L67

About my suggestion: it was made 6 month ago, since my opinion changed about the output (c.f. the conclusion point in my last comment)

[stof]

oading the command's dependencies when using help should still be possible: in some cases you want to retrieve information from services to use it as option default value.

I'm not saying it should not. I'm even saying the opposite. I consider it is valid to initialize the command dependencies when displaying the help for this command, which is why only the command name needs to be lazy, not the whole config.

Regarding my comment saying your suggestion does not make sense, it was a reply to @wouterj saying he likes this suggestion

[gnugat]

Ok.

To come back to your suggestion: how would you manage aliases? And name resolution when using shortcuts?

[stof]

@gnugat the name resolution for shortcuts is not done by the command. It is done by the application based on the known names. For aliases, it simply means that the DI tag needs to provide all command names in its attribute (i.e. the real name and all aliases), not only the main name

[DerManoMann]

I agree that separation of console and commands would be a good thing. I also strongly disagree with the idea of pressing the console into a request/response schema.
Most of the commands I work on are pure CL commands, so a concept of request/response seems very artificial.
👎

[jeremyFreeAgent]

@stof you're right about the alias in the DI tag, but also the name must be there since the name is added in the Application when adding the Command with the add method.
In Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\AddConsoleCommandPass we have to check if the tag has at least the command name to be lazyable.

[linaori]

I would be very happy with proper DI support in the console component. The console/command is nothing more than a controller for the CLI.

[gnugat]

@iltar you can already use the DI with the console, there's no need to couple them

[linaori]

@gnugat I think you are confusing DI with the DIC. You don't couple DI on something, it's a way of programming. I want to inject my dependencies in the console/command and not have it know about the DIC. By having DI you can typehint your objects so you know you always have the right thing. With container::get you'd have to call instanceof to be sure.

Edit: phone autocomplete mistakes.

[gnugat]

@iltar Ok I thought you were saying that we needed to couple the DIC with console. Let me rephrase my statement: you can already have proper DI with the console, you have full control on your command constructors. They only thing you need to remember is to call parent__construct

[linaori]

@gnugat and that's the part that's eating me away. You need to depend on the Command class. In my opinion the following steps should be taken to normalize the controller and command:

• Controllers should be taggable like Command as a Service to recognize them instead of relying on auto search (possibly a small optimization)
• Command as a Service should make you independent of Command and use this proposal to generate the "action" parameters such as input and output: generic solution to injecting action parameters by type hint #10710

[fabpot]

Closing as I won't make the Console component looks like HttpKernel.