Capture output goes to terminal by default #2741
Conversation
Capture and console.capture() accept kwargs, where you may specify echo=False to silence output being sent to terminal.
rich/console.py
Outdated
| @@ -327,9 +327,10 @@ class Capture: | |||
| console (Console): A console instance to capture output. | |||
| """ | |||
|
|
|||
| def __init__(self, console: "Console") -> None: | |||
| def __init__(self, console: "Console", **kwargs) -> None: | |||
There was a problem hiding this comment.
Don't see any reason to use kwargs here. It breaks typing.
There was a problem hiding this comment.
hmm guessing you're suggesting a default arg instead - def __init__(..., echo: bool=True)?
rich/console.py
Outdated
| @@ -342,6 +343,8 @@ def __exit__( | |||
| exc_tb: Optional[TracebackType], | |||
| ) -> None: | |||
| self._result = self._console.end_capture() | |||
| if self._echo: | |||
| self._console.out(self._result) | |||
There was a problem hiding this comment.
This is going to run captured input through print again, which may modify the result.
There was a problem hiding this comment.
right, guess I'll invoke print directly then.
| @@ -379,22 +379,33 @@ def test_capture(): | |||
| console.print("Hello") | |||
| assert capture.get() == "Hello\n" | |||
|
|
|||
| with console.capture(echo=False) as capture: | |||
There was a problem hiding this comment.
This doesn't test that echo is working.
tinvaan
force-pushed
the
fix/capture-to-terminal
branch
from
9c1adad to
7ead408
Compare
|
@willmcgugan I've addressed the review comments, but I see locally that my change breaks the However, printing the test variable to console shows the text styled as expected("styled content: Hello World") but we're asserting for a plain string in the tests. Shouldn't it be the other way round? What am I missing?
Wanted to check and confirm before modifying any test logic. |
|
@willmcgugan poke? |
|
There are other considerations you may not be aware of. You might want to leave this one to us. |
Ok, I'll keep the PR open for anyone who might want to attempt a fix. Thanks! |
Capture and console.capture() accepts a kwargs, where you may specify echo=False to silence output being sent to terminal.
Type of changes
Checklist
Description
Attempts a fix for #2172 by implementing @willmcgugan's suggestion