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

[8.x] Expose the underlying output implementation for OutputStyle #39099

Merged
merged 1 commit into from
Oct 5, 2021

Conversation

jhdxr
Copy link
Contributor

@jhdxr jhdxr commented Oct 5, 2021

As requested and discussed in laravel/ideas#2064, some symfony features (appendRow for tables, or multi progress bar) requires output implementation other than BufferedOutput. However, since laravel hides the $output, there is no way for users to do so unless rewrting everything related to console output.

There was an attemp (#31426) to support it but it broke many tests and got reverted finally (4094d78), because that PR also changes the consturction method of Table.

So instead of rewriting related codes or forwarding/proxying the calls, this PR proposes to simply expose the $output by adding a getter to allow advanced users to implment those feature without rewriting the whole thing, and at the same time we don't have to worry about it breaks anything.

@taylorotwell taylorotwell merged commit 36b403d into laravel:8.x Oct 5, 2021
@jhdxr
Copy link
Contributor Author

jhdxr commented Oct 5, 2021

In case this PR get rejected or merged but reverted, or for people who stick with the old versions, here is a workaround, you can use Reflection to get any private attribute, including $output here:

        $output = $this->getOutput(); //this is OutputStyle
        $rOutput = new \ReflectionClass($output);
        $pOutput = $rOutput->getProperty('output');
        $pOutput->setAccessible(true);
        $rawOutput = $pOutput->getValue($output); //this is the underlying implmenetation of OutputInterface

Please note reflection is usually considered as slow, so this is just a workaround.

victorvilella pushed a commit to cdsistemas/framework that referenced this pull request Oct 12, 2021
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.

2 participants