[Console] ConsoleOuput::isDecorated is detected from STDOUT and STDERR #13449
Conversation
8f59140 to
df1ee7f
Compare
| protected function hasColorSupport() | ||
| { | ||
| // emulate posix_isatty(STDOUT) !== posix_isatty(STDERR) | ||
| return ($this->expected = ! parent::hasColorSupport()); |
There was a problem hiding this comment.
Can you remove the parenthesis and the space after !?
df1ee7f to
688db71
Compare
|
Removed space and parenthesis, then amended. |
|
I think #13661 fixes this? Can you check? :-) |
|
Thank you @alcohol for the ping. I ping in also @Seldaek since helped you with the fix. Your solutions allow to have a decorated stderr with undecorated stdout, but I still prefer this PR. The main reason is that the formatter is injected, so it is not a good idea to clone it. The application (not only symfony) may define a non-clonable custom formatter, or a formatter with runtime setting. So I'm 👎 on fixing the bug by cloning, since it is potentially a BC. That said, I agree with you that stderr should be "promoted" to first class object in console. The source of the bug is that if you pass |
|
I think there are two possible fixes:
This PR would just do the detection on stdout and IMO it's unacceptable that either stderr or stdout gets colors if they are not capable of receiving them. |
Your right, assuming that "stdout rules" is not enough. I'll change the commit to handle it as you suggest. Then the decision is up to the maintainers :) |
|
Ok, I've amended the commit with the changes suggested by @Seldaek . This is no longer a one-line fix, but there are no BC. The auto-detection is done on both streams and then |
688db71 to
e8f300c
Compare
…corated when both stderr and stdout support colors (Seldaek) This PR was merged into the 2.3 branch. Discussion ---------- [Console] Ensure the console output is only detected as decorated when both stderr and stdout support colors | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #10592 #13449 | License | MIT | Doc PR | This is a simplified version of #13661 which does not create any issues with having two decorators but merely ensures that if either STDERR **or** STDOUT has colors disabled, then both will have decoration disabled. It's not a perfect solution but it's better than having both enabled as this breaks things. And I don't think we can come to a better solution without api changes. Commits ------- f3d8444 [Console] Ensure the console output is only detected as decorated when both stderr and stdout support colors
In a Symfony command, the
OutputInterfacehas wrong result forisDecoratedin this very simple scenario:$ app/console vendor:command > outputHere
outputis a regular file, so$output->isDecorated()should returnfalse, but it returnstrue. This happen because, the the automatic detection is done onstdoutfirst, and then overridden by a second detection onstderr(which is tty in the example above).I've added a test which (hopefully) make clear the scenario.