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

Check if stderr is a terminal for error messages #1766

Merged
merged 1 commit into from
Jan 18, 2024
Merged

Conversation

vkleen
Copy link
Contributor

@vkleen vkleen commented Jan 18, 2024

Previously we checked whether stdout is a terminal to make our coloring decision, and then we proceeded to happily assume the same state applies to stderr for actual error messages. However, if someone redirects stdout to capture evaluation results, stderr may still be a terminal and should receive colored output in this case. This PR implements this behavior and makes the choice of which output stream to check explicit.

Prveiusly we checked whether `stdout` is a terminal to make our coloring
decision, and then we proceeded to happily assume the same state applies
to `stderr` for ad mctual error messages. However, if someone redirects
`stdout` to capture evaluation results, `stderr` may still be a terminal
and should receive colored output in this case. This PR omplements this
behavior and makes the choice of which output stream to check explicit.
@vkleen vkleen requested review from jneem and yannham January 18, 2024 13:40
@github-actions github-actions bot temporarily deployed to pull request January 18, 2024 13:44 Inactive
@vkleen vkleen added this pull request to the merge queue Jan 18, 2024
Merged via the queue into master with commit 22a0274 Jan 18, 2024
5 checks passed
@vkleen vkleen deleted the fix/stderr-color branch January 18, 2024 14:22
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