Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[6.x] Split specifyParameter() to external trait #31254

Merged
merged 2 commits into from
Jan 30, 2020

Conversation

crynobone
Copy link
Member

While building orchestra/canvas, I found out that while it would be nice
to use Illuminate\Console\Command the requirements to have Laravel (or
at least the Container) configure make it slightly harder to build PHP
CLI code based on symfony console with a little Laravel flavour.

    /**
     * Configure the command options.
     *
     * @return void
     */
    protected function configure()
    {
        $this->ignoreValidationErrors();

        $this->setName($this->name)
                ->setDescription($this->description)
                ->addArgument('name', InputArgument::REQUIRED, "The name of the {$this->fileType}");

        $this->specifyParameters();
    }

Signed-off-by: Mior Muhammad Zaki crynobone@gmail.com

While building orchestra/canvas, I found out that while it would be nice
to use Illuminate\Console\Command the requirements to have Laravel (or
at least the Container) configure make it slightly harder to build PHP
CLI code based on symfony console with a little Laravel flavour.

Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com>
@GrahamCampbell GrahamCampbell changed the title [6.x] Split specifyParameter() to external trait. [6.x] Split specifyParameter() to external trait Jan 28, 2020
if ($arguments instanceof InputArgument) {
$this->getDefinition()->addArgument($arguments);
} else {
call_user_func_array([$this, 'addArgument'], $arguments);
Copy link
Member

Choose a reason for hiding this comment

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

The trait needs to have getDefinition, addArgument and addOption as abstract methods.

Copy link
Member Author

@crynobone crynobone Jan 28, 2020

Choose a reason for hiding this comment

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

addArgument and addOption both will have scalar type-hint in 5.0. Does Laravel want to have this?

Copy link
Member

Choose a reason for hiding this comment

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

Laravel 6 requires Symfony console 4, so no typehints needed. For Laravel 7, yes, we'll need to match the symfony method signature, since that needs Symfony console 5.

Copy link
Member

Choose a reason for hiding this comment

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

The same as some of Laravel's internals has typehints to match phpunit's stuff.

@taylorotwell taylorotwell merged commit 0d81e50 into laravel:6.x Jan 30, 2020
@crynobone crynobone deleted the console-has-parameters branch February 1, 2020 01:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants