Skip to content

[Console] Review console.ERROR related behavior #22441

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

Merged
merged 1 commit into from
Apr 26, 2017

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Apr 14, 2017

Q A
Branch? 3.3
Bug fix? yes
New feature? yes
BC breaks? no
Deprecations? yes
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

This PR is a follow up of #18140 that I wanted to do since a few weeks.
It enhances this work with fixes and behavior changes.
It embeds #22435 and resolves issues like the one described in #20808.

use Symfony\Component\Console\Event\ConsoleTerminateEvent;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;

/**
* @author James Halsall <james.t.halsall@googlemail.com>
* @author Robin Chalas <robin.chalas@gmail.com>
*
* @deprecated since version 3.3, to be removed in 4.0. Use ErrorListener instead.
Copy link
Member

Choose a reason for hiding this comment

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

This listener has been added in 3.3 (#21003), can be renamed directly :)

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed, thanks

@nicolas-grekas
Copy link
Member Author

👍 also :)

return parent::getException();
$r = new \ReflectionProperty($this->error, 'code');
$r->setAccessible(true);
$r->setValue($this->error, $this->exitCode);
Copy link
Member

Choose a reason for hiding this comment

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

why altering the exception ?

Copy link
Member Author

Choose a reason for hiding this comment

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

because the other part of the exception handling uses getCode as the way to set the exit status code from an exception

parent::__construct($command, $input, $output);

$this->exception = $exception;
$this->setException($exception);
Copy link
Member

Choose a reason for hiding this comment

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

useless change

@fabpot
Copy link
Member

fabpot commented Apr 26, 2017

Thank you @nicolas-grekas.

@fabpot fabpot merged commit a7c67c9 into symfony:master Apr 26, 2017
fabpot added a commit that referenced this pull request Apr 26, 2017
…as-grekas)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[Console] Review console.ERROR related behavior

| Q             | A
| ------------- | ---
| Branch?       | 3.3
| Bug fix?      | yes
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

This PR is a follow up of #18140 that I wanted to do since a few weeks.
It enhances this work with fixes and behavior changes.
It embeds #22435 and resolves issues like the one described in #20808.

- makes ConsoleErrorEvent *not* extend the deprecated ConsoleExceptionEvent
- replace ConsoleErrorEvent::markErrorAsHandled by ConsoleErrorEvent::setExitCode
- triggers the deprecation in a more appropriate moment
- renames ExceptionListener to ErrorListener
- tweaks the behavior in relation to  #22435

Commits
-------

a7c67c9 [Console] Review console.ERROR related behavior
@nicolas-grekas nicolas-grekas deleted the console-error branch April 29, 2017 16:39
@fabpot fabpot mentioned this pull request May 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants