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

[Console] Do not display short exception trace for common console exceptions #24131

Merged
merged 1 commit into from Sep 7, 2017

Conversation

yceruto
Copy link
Member

@yceruto yceruto commented Sep 7, 2017

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

console_before

I'd like reconsider the new error output with short exception trace always displayed at top, IMHO it's annoying for common exceptions (there is not real debugging reason, the message is clear enough) such as Symfony\Component\Console\Exception\* which have an impact into current CLI applications.

However, I'm proposing display it for unexpected exceptions or if verbosity is enabled:

console

Note @nicolas-grekas's #21414 (comment) is still covered.

Copy link
Member

@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

👌 for me.

Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

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

Makes sense

@chalasr
Copy link
Member

chalasr commented Sep 7, 2017

Thank you @yceruto.

@chalasr chalasr merged commit 47b1106 into symfony:3.4 Sep 7, 2017
chalasr pushed a commit that referenced this pull request Sep 7, 2017
…mon console exceptions (yceruto)

This PR was merged into the 3.4 branch.

Discussion
----------

[Console] Do not display short exception trace for common console exceptions

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

![console_before](https://user-images.githubusercontent.com/2028198/30173516-edb9e42c-93c5-11e7-882e-f8b0335387b3.png)

I'd like reconsider [the new error output][1] with short exception trace always displayed at top, IMHO it's annoying for common exceptions (there is not real debugging reason, the message is clear enough) such as `Symfony\Component\Console\Exception\*` which have an impact into current CLI applications.

However, I'm proposing display it for unexpected exceptions or if verbosity is enabled:

![console](https://user-images.githubusercontent.com/2028198/30173085-94322636-93c4-11e7-81a6-bba807910e62.png)

Note @nicolas-grekas's #21414 (comment) is still covered.

 [1]: #21414

Commits
-------

47b1106 Do not display short exception trace for built-in console exceptions
@chalasr chalasr removed the request for review from nicolas-grekas September 7, 2017 20:11
@chalasr chalasr added this to the 3.4 milestone Sep 7, 2017
@yceruto yceruto deleted the console_error_output branch September 7, 2017 20:44
This was referenced Oct 18, 2017
nicolas-grekas added a commit that referenced this pull request May 4, 2018
…rom exception messages in Symfony's commands (yceruto)

This PR was merged into the 3.4 branch.

Discussion
----------

[Console] By default hide the short exception trace line from exception messages in Symfony's commands

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

After #24131 this was in my contribution list since then.

Maybe it should be taken as a good practice when we build console commands, **use the exception classes of the Console component as much as possible to show a better message style to the end user**.

(See the before/after effect in the referenced PR)

Commits
-------

11f3c45 Hide short exception trace by default
symfony-splitter pushed a commit to symfony/framework-bundle that referenced this pull request May 4, 2018
…rom exception messages in Symfony's commands (yceruto)

This PR was merged into the 3.4 branch.

Discussion
----------

[Console] By default hide the short exception trace line from exception messages in Symfony's commands

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

After symfony/symfony#24131 this was in my contribution list since then.

Maybe it should be taken as a good practice when we build console commands, **use the exception classes of the Console component as much as possible to show a better message style to the end user**.

(See the before/after effect in the referenced PR)

Commits
-------

11f3c455d4 Hide short exception trace by default
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.

None yet

3 participants