8000 [Console] Design issue with decorator and OutputFormater · Issue #10592 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Console] Design issue with decorator and OutputFormater #10592

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.

Sign up for GitHub

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

Jump to bottom
Closed
lyrixx opened this issue Mar 31, 2014 · 11 comments
Closed

[Console] Design issue with decorator and OutputFormater #10592

lyrixx opened this issue Mar 31, 2014 · 11 comments
Labels
Console

Comments

@lyrixx
Copy link
Member
lyrixx commented Mar 31, 2014

Current state:

class ConsoleOutput extends StreamOutput implements ConsoleOutputInterface
{
    public function __construct($verbosity = self::VERBOSITY_NORMAL, $decorated = null, OutputFormatterInterface $formatter = null)
    {
        $outputStream = 'php://stdout';
        if (!$this->hasStdoutSupport()) {
            $outputStream = 'php://output';
        }

        parent::__construct(fopen($outputStream, 'w'), $verbosity, $decorated, $formatter);

        $this->stderr = new StreamOutput(fopen('php://stderr', 'w'), $verbosity, $decorated, $formatter);
    }
}

As you can see, the $formatter is shared between STDOUT and STDERR ouput. In symfony, everything works fine, but in composer this does not work as the formatter is shared. It does not work in composer because composers overrides the method Application::run. (ref).

So, if you ran composer | cat -, STDOUT is not a tty. But symfony and/or composer thinks STDOUT is a tty. And so the formatter is decorated. And so their is some color (ansi) on the output. If you execute app/console | cat -, their is not color on the output, which is the good behavior.

This happen in composer because Formatter->setDecorated() is called twice, the first time for STDOUT (false) and the second time with for STDERR (true).
@stof stof added the Console label Mar 31, 2014
@cordoval
Copy link
Contributor

is this issue related bldr-io/bldr#115 ? We actually want to output color. Currently we see the ->getOutput is getting us no colors when there should be. Any hint? cc/ @romainneutron

@lyrixx
Copy link
Member Author
lyrixx commented Sep 22, 2014

may be. The initial issue was in composer.

@cordoval
Copy link
Contributor

@lyrixx yes, but your issue is that you did not have color or that you did have color when you shouldn't have?

My issue is that I am doing

$output = $this->getOutput();
$process->start(
            function ($type, $buffer) use ($output) {
                $output->write($buffer);
            }
);

while ($process->isRunning()) {
}

and getting no color

@lyrixx
Copy link
Member Author
lyrixx commented Sep 22, 2014

don't remember

@cordoval
Copy link
Contributor

@lyrixx I got some light shed on my problem, it is more related to phpspec and behat because phpunit does output with color without problems when i run it inside my process. What i am wondering is why Process output gets stripped of these colors when i output it. What is phpspec or behat doing differently is that they are based on Symfony App console one way or the other, so this emphasizes the fact that something is very wrong... 🐱

@cordoval
Copy link
Contributor

nvm i guess it was all in the guessing on the colors inside a process --colors solved it 👍

@stof
Copy link
Member
stof commented Sep 25, 2014

Yeah, the reason PHPUnit gives you colors is because it does not care whether your console supports colors. If you configure it with colors and they are not supported, you will just get an ugly output because of the ANSI sequences being displayed.
A subprocess is not running in a tty by default (there is a way to do this in recent versions. I don't remember which release added it), which is why you were getting no colors

@cordoval
Copy link
Contributor

well the only problem on my side was that i was running bldr, bldr in itself gives you colors, however it wraps in a subprocess the execution of its calls which could be phpspec or behat. So I was not passing --ansi or --colors to force them so there was no color on their outputs. When I did force it with these flag options on the calls then voila i started to get colors 👍 . I think the initial problem for when @lyrixx was wrapping a cli application with the app console component is still valid and more of a design problem than what I was talking about. And still valid.

@remicollet
Copy link
Contributor

I think a possible fix to this issue is:

-    $this->stderr = new StreamOutput(fopen('php://stderr', 'w'), $verbosity, $decorated, $this->getFormatter());
+    $this->stderr = new StreamOutput(fopen('php://stderr', 'w'), $verbosity, $this->isDecorated(), $this->getFormatter());

@remicollet remicollet mentioned this issue Jan 6, 2015
Closed
@remicollet
Copy link
Contributor

Workaround to this issue

User way

$ thephpapp  2>&1 | cat

Dev way :

-   $output = new ConsoleOutput();
+   $output = new StreamOutput(STDOUT);

@alcohol
Copy link
Contributor
alcohol commented Feb 11, 2015

Related #13661

@Seldaek Seldaek mentioned this issue Sep 12, 2015
Merged
fabpot added a commit that referenced this issue Sep 13, 2015
@fabpot
bug #15771 [Console] Ensure the console output is only detected as de…
e2156d7 
…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      
6AB4
       | 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
@fabpot fabpot closed this as completed Sep 13, 2015
@wouterj wouterj mentioned this issue Nov 4, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Console
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@alcohol @fabpot @remicollet @cordoval @lyrixx @stof
0