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

BUGFIX: Output capturing for console respects the diable output setting #19

Merged
merged 2 commits into from
Mar 14, 2021

Conversation

Fischer-Simon
Copy link
Contributor

Also tables where not captured and always printed to the console.

@hlubek
Copy link
Contributor

hlubek commented Oct 16, 2020

This is indeed a regression from 1fdf97b, so could we please merge this?

Regarding the TODO and table output: ideally we could inject a different output instance in ConsoleOutput that captures all console output, so everything "just works" without advicing lots of different methods.

hlubek added a commit to hlubek/behat that referenced this pull request Oct 16, 2020
If a command is run and throws an exception it must not be catched. Better use `finally` to just make sure output is enabled (which is always enabled anyway until neos#19 is merged).
Copy link
Member

@albe albe left a comment

Choose a reason for hiding this comment

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

Indeed a boolean swap regression! Thanks a lot for finding and submitting this patch!

Regarding the TODO and table output: ideally we could inject a different output instance in ConsoleOutput that captures all console output, so everything "just works" without advicing lots of different methods.

That would be cool, so if anyone is up for digging into that, feel free to submit a PR and tag me for review!

@albe albe changed the title The output capturing for console output did not respect the diable output setting. BUGFIX: Output capturing for console respects the diable output setting Mar 14, 2021
@albe albe merged commit 628a4be into neos:master Mar 14, 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.

3 participants