Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Console] Add command resolver (proof of concept)#16438
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
use anull comparison instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
it should benull !== $commandResolver ? and parentheses is useless here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Wrong indent in whole file. Use 4 spaces not 2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Sorry for that. I will fix it.
funivan commentedNov 12, 2015
@stof Hello. Can you help me? I can`t pass all tests of this PR at CI services (In local environment all tests are green). |
TomasVotruba commentedMar 3, 2016
Very nice PR! Tests for Console are passing well. It fails on Symfony/Bridge/ProxyManager package, not sure why. |
funivan commentedMar 3, 2016
@TomasVotruba yep and I don`t know how to resolve this. Is there any way to rerun this tests? |
DavidBadura commentedMar 3, 2016
with a rebase 😉 |
funivan commentedJul 22, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Hi 2 all. It is easier to create new pull request and implement the same logic. |
| $this->helperSet =$this->getDefaultHelperSet(); | ||
| $this->definition =$this->getDefaultInputDefinition(); | ||
| $this->commandResolver =$commandResolver !==null ?$commandResolver :newCommandResolver(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think we use$this->commandResolver = $commandResolver ?: new CommandResolver()
nicolas-grekas commentedJul 11, 2017
This PR was merged into the 3.4 branch.Discussion----------[Console] Add support for command lazy-loading| Q | A| ------------- | ---| Branch? | 3.4| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#12063#16438#13946#21781| License | MIT| Doc PR | todoThis PR adds command lazy-loading support to the console, based on PSR-11 and DI tags.(#12063 (comment))Commands registered as services which set the `command` attribute on their `console.command` tag are now instantiated when calling `Application::get()` instead of all instantiated at `run()`.__Usage__```yamlapp.command.heavy: tags: - { name: console.command, command: app:heavy }```This way private command services can be inlined (no public aliases, remain really private).With console+PSR11 implem only:```php$application = new Application();$container = new ServiceLocator(['heavy' => function () { return new Heavy(); }]);$application->setCommandLoader(new ContainerCommandLoader($container, ['app:heavy' => 'heavy']);```Implementation is widely inspired from Twig runtime loaders (without the definition/runtime separation which is not needed here).Commits-------7f97519 Add support for command lazy-loading
This PR was merged into the 3.4 branch.Discussion----------[Console] Add support for command lazy-loading| Q | A| ------------- | ---| Branch? | 3.4| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |symfony/symfony#12063symfony/symfony#16438symfony/symfony#13946 #21781| License | MIT| Doc PR | todoThis PR adds command lazy-loading support to the console, based on PSR-11 and DI tags.(symfony/symfony#12063 (comment))Commands registered as services which set the `command` attribute on their `console.command` tag are now instantiated when calling `Application::get()` instead of all instantiated at `run()`.__Usage__```yamlapp.command.heavy: tags: - { name: console.command, command: app:heavy }```This way private command services can be inlined (no public aliases, remain really private).With console+PSR11 implem only:```php$application = new Application();$container = new ServiceLocator(['heavy' => function () { return new Heavy(); }]);$application->setCommandLoader(new ContainerCommandLoader($container, ['app:heavy' => 'heavy']);```Implementation is widely inspired from Twig runtime loaders (without the definition/runtime separation which is not needed here).Commits-------7f97519 Add support for command lazy-loading
Hello. This is simple proof of concept for command resolver.
Why?
On huge projects we should always initialize all commands, even if they are not used.
Some users want lazy-loading of commands. And there are a lot of ways how to do that. Store cache in files, db, initialize commands by unique id etc..
How?
This PR introduce new interface
CommandResolverInterface. Any user can implement logic and inject it to the application. With this interface developer can create commandsby requestBC ?
This interface is softly injected to this application. All tests pass. By default
CommandResolveris simple holder for commands.Current state
There are one problem: when we add
Commandto application, application add this command only if it is available. This logic is not covered inCommandResolverInterface. We put all responsibility to developer which Commands are available to application.If it will be interesting to the team I can polish it (fix code style, possible change names of interface, write documentation etc.)