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

[8.x] Remove an useless check in Console Application class #40145

Merged
merged 1 commit into from
Dec 23, 2021

Conversation

seriquynh
Copy link
Contributor

What I do?

I create this PR as a re-open PR of #40133

Why I do this?

@GrahamCampbell said I was incorrect but I was NOT incorrect.

  1. If the condition ! isset($callingClass) && empty($parameters) is true, this line below will be executed and the $input will be an instance of StringInput.
$command = $this->getCommandName($input = new StringInput($command));
  1. Otherwise, these two lines will be executed and the $input will be an instance of ArrayInput.
array_unshift($parameters, $command);

$input = new ArrayInput($parameters);

Conclusion

To conclude, there are no cases of this context in which the $input isn't set, it's always set. Therefore, the check $input ?? null is absolutely useless.

If the Laravel team rejects this PR, it will be your final decision and I understand. But rejecting this PR doesn't mean my PR is incorrect. It's reasonable.

Let's see the method more details:

protected function parseCommand($command, $parameters)
{
    if (is_subclass_of($command, SymfonyCommand::class)) {
        $callingClass = true;

        $command = $this->laravel->make($command)->getName();
    }

    if (! isset($callingClass) && empty($parameters)) {
        $command = $this->getCommandName($input = new StringInput($command));
    } else {
        array_unshift($parameters, $command);

        $input = new ArrayInput($parameters);
    }

    return [$command, $input ?? null];
}

@seriquynh
Copy link
Contributor Author

@driesvints Could you take a look at this please!

@taylorotwell taylorotwell merged commit 38f9113 into laravel:8.x Dec 23, 2021
@seriquynh seriquynh deleted the remove-useless-check branch December 24, 2021 03:00
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.

3 participants