fop_console
fop_console copied to clipboard
Fix "employee" option not found
Fixes #255
I left the shortcut as defined, but I was not able to make it work.
-em=1 --em=1 -em1 -em 1 --em 1
Whatever I tried, I had "option not found" or "The file "/var/www/html/app/config/config_***.yml" does not exist because it falls under the -e
option
My fix is not good. It fixed my use case but it does not work with existing commands because they would have to call "parent::configure()" in every command class.
Moved the addOption to constructor instead of configure().
The configure()
method is implemented by Commands to set name and description, without calling the parent method, like
protected function configure(): void
{
$this->setName('fop:module:generate')
->setDescription('Scaffold new PrestaShop module')
;
}
Therefore, doing the addOption
within the configure method from the parent Command class means that the addOption is never done.
It worked for me because I did my own command and called the parent method like parent::configure()
.
Before
php bin/console fop:employee:list --employee=1
ERROR [console] Error thrown while running command "fop:employee:list --employee=1". Message: "The "--employee" option does not exist."
I'm really sorry for the late help. I'll try to be more active on the project from now on.
You're right, we have to call parent configure()
method in every command we create. By the way, I prefer to call parent method first personally, before the child statements.
Also, the id_shop
and id_shop_group
options seems to be planned but not fully implemented too... We could add them too!
Are you still ready to continue the pull request?
Hello ! Thanks for your reply :)
You're right, we have to call parent configure() method in every command we create. By the way, I prefer to call parent method first personally, before the child statements.
I manage to avoid modifying all the commands by adding the option into the constructor. Not great but working. I am ready to continue on the pull request, no problem. I just have to be sure what to do :)
Hello ! Thanks for your reply :)
You're right, we have to call parent configure() method in every command we create. By the way, I prefer to call parent method first personally, before the child statements.
I manage to avoid modifying all the commands by adding the option into the constructor. Not great but working. I am ready to continue on the pull request, no problem. I just have to be sure what to do :)
I prefer to configure the command in the dedicated method if possible. I'm not against modifying all the commands, but rather the opposite: we should have called the parent method since the beginning of the project imo. So feel free to go ahead and add the parent call in all the commands ;)
I didn't test, but lgtm.