[Console] Allow to catch CommandNotFoundException #22181
Conversation
| @@ -27,7 +27,7 @@ class ConsoleErrorEvent extends ConsoleExceptionEvent | |||
| private $error; | |||
| private $handled = false; | |||
|
|
|||
| public function __construct(Command $command, InputInterface $input, OutputInterface $output, $error, $exitCode) | |||
| public function __construct(Command $command = null, InputInterface $input, OutputInterface $output, $error, $exitCode) | |||
There was a problem hiding this comment.
It's always weird to have optional argument before required one. Could we do something for that ?
There was a problem hiding this comment.
Agreed, arguments have been reordered
2337015 to
0ff0123
Compare
|
ping @wouterj |
| $exception = $exitCode; | ||
| $exitCode = $tmpExitCode; | ||
|
|
||
| @trigger_error(sprintf('Passing an instance of %s as first argument of %s() is deprecated since version 3.3. Pass it as 5th argument instead.', Command::class, __METHOD__), E_USER_DEPRECATED); |
There was a problem hiding this comment.
This deprecation is useless imo. People creating the class as ($command, $input, $output) already get a deprecation, as the class shouldn't be initialized (ConsoleErrorEvent should be initialized instead).
There was a problem hiding this comment.
Indeed, deprecation removed.
fc50aef to
ca50cf9
Compare
| @@ -27,12 +25,21 @@ class ConsoleExceptionEvent extends ConsoleEvent | |||
| private $exception; | |||
| private $exitCode; | |||
|
|
|||
| public function __construct(Command $command, InputInterface $input, OutputInterface $output, \Exception $exception, $exitCode, $deprecation = true) | |||
| public function __construct($input, $output, $exception, $exitCode, $command = null, $deprecation = true) | |||
There was a problem hiding this comment.
Given that the class is deprecated anyway I would just allow $command to be null instead of shifting constructor arguments.
There was a problem hiding this comment.
Thanks, I wasn't sure. Fixed.
ca50cf9 to
b21ce85
Compare
|
Ready, build failure unrelated. |
|
Bump, it will be harder to make this change after 3.3 (would involve deprecations). |
|
Thank you @chalasr. |
…lasr) This PR was merged into the 3.3-dev branch. Discussion ---------- [Console] Allow to catch CommandNotFoundException | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #22144 (comment) | License | MIT | Doc PR | n/a Basically reverts #22144, making the command argument optional in console.error event instead, so that `CommandNotFoundException` can be handled as any other console error. Commits ------- b21ce85 [Console] Allow to catch CommandNotFoundException
Basically reverts #22144, making the command argument optional in console.error event instead, so that
CommandNotFoundExceptioncan be handled as any other console error.