Kdyby / Console

Symfony Console integration for Kdyby components
https://packagist.org/packages/kdyby/console
Other
52 stars 54 forks source link

Container is not available in Command::configure #58

Closed Fuco1 closed 8 years ago

Fuco1 commented 8 years ago

I would like to have the parameters section available already in configure so I can provide sensible defaults based on the configuration (from config.neon).

Unfortunatelly, the commands are configured from their constructor and so the helper set is not yet injected => we can't call the container helper (that only happens in Application::add but that is too late as it accepts the command as already constructed parameter).

The way I got around this was to extend the command and pass it the container via constructor. It's not pretty but it works.

Do you guys have some better ideas about how to solve this chicken&egg problem, best in some automagic way so the user doesn't have to deal with each command manually.

Here's the sample code

console:
    commands:
        - App\Console\AppCommand(@container)
class AppCommand extends Command
{
    private $container;

    // TODO: prasarna, ale by default $container este nie je pristupny v configure
    public function __construct(\Nette\DI\Container $container) {
        $this->container = $container;
        parent::__construct(); // calls $this->configure()
    }

    protected function configure()
    {
        $parameters = $this->container->getParameters()['config'];
        $this->addOption(
            'destination-db-host', 'e', InputOption::VALUE_REQUIRED,
            "destination db host",
            $parameters['defaultDestinationDbHost']
        );
        //
    }
}
foglcz commented 8 years ago

As a hint: you DON'T need to define configure() of you set up everything in constructor. On Jun 3, 2016 12:39 PM, "Matus Goljer" notifications@github.com wrote:

I would like to have the parameters section available already in configure so I can provide sensible defaults based on the configuration (from config.neon).

Unfortunatelly, the commands are configured from their constructor and so the helper set is not yet injected => we can't call the container helper (that only happens in Application::add but that is too late as it accepts the command as already constructed parameter).

The way I got around this was to extend the command and pass it the container via constructor. It's not pretty but it works.

Do you guys have some better ideas about how to solve this chicken&egg problem, best in some automagic way so the user doesn't have to deal with each command manually.

Here's the sample code

console: commands:

  • App\Console\AppCommand(@container)

class AppCommand extends Command{ private $container; // TODO: prasarna, ale by default $container este nie je pristupny v configure public function construct(\Nette\DI\Container $container) { $this->container = $container; parent::construct(); // calls $this->configure() } protected function configure() { $parameters = $this->container->getParameters()['config']; $this->addOption( 'destination-db-host', 'e', InputOption::VALUE_REQUIRED, "destination db host", $parameters['defaultDestinationDbHost'] ); // }}

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Kdyby/Console/issues/58, or mute the thread https://github.com/notifications/unsubscribe/AAu3Xz--Q2wjHGc53WCcJUBg2cq3u5eMks5qIBJwgaJpZM4ItcxR .

Fuco1 commented 8 years ago

Defining the options in the constructor doesn't really help though, does it? I still don't have the container there.

Or do you mean I should set up the options/arguments in the execute method?

foglcz commented 8 years ago

Wouldn't this work?

edit: Sorry, forgot that Command requires a first parameter, which is the name of the command. It's a major piss at the architectore of symfony console vs documentation :'( Fixed.

console:
    commands:
        - App\Console\AppCommand('', %config%)
class AppCommand extends Command
{
    public function __construct($name = '', array $parameters) {
        parent::__construct('your:command'); // <-- interestingly, this is alternative to $this->setName()
        $this->addOption(
            'destination-db-host', 'e', InputOption::VALUE_REQUIRED,
            "destination db host",
            $parameters['defaultDestinationDbHost']
        );
    }
}
foglcz commented 8 years ago

... either way what's important is - if you can pass @container as a service, you should be able to pass the parameters directly.

Everything you do in configure() you can also do in __construct, there's really no difference as construct call configure() anyway.

Fuco1 commented 8 years ago

My god I feel so stupid right now :D

Yea that should be good for my use-case.

Fuco1 commented 8 years ago

@foglcz right, I see what you ment. Yea, passing the config directly just did not occur to me at all, heh.

Good to close I guess. Thanks for the help!

foglcz commented 8 years ago

happens to the best of us :) This is my personal approach to sf console so it's obv to me - but it's certainly not documented this way in sf docs, for some reason.

Happy to help! :) :+1: