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

fix: Target::Pipe respects WriteStyle #275

Closed
wants to merge 3 commits into from

Conversation

miraclx
Copy link

@miraclx miraclx commented Aug 9, 2023

Resolves #274.

The old behavior made it impossible to retain colors in custom sinks.

With this patch, the default behavior assumes non-color compliance with the sink. But this can be overridden with WriteStyle::Always.

@matthiasbeyer
Copy link
Member

What exactly does this PR change? I only see renamed variables, a cast that I don't think is necessary and some if {} else {} changed to then_some()?

Comment on lines 111 to 114
has_alt_target: matches!(
self.alt_target,
Some(WritableTarget::Stderr | WritableTarget::Stdout)
),
Copy link
Author

@miraclx miraclx Aug 9, 2023

Choose a reason for hiding this comment

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

@matthiasbeyer, this. The variable rename should make sense in this context, and since I was already modifying the lines, I figured a rewrite to the then_some syntax was ideal.

As for the cast, rust-analyzer kept hinting at a type conflict that rustc didn't flag. Adding an arbitrary typecast was the quickest way to remove the error.

@epage
Copy link
Contributor

epage commented Jan 18, 2024

This is superseded by #298

@epage epage closed this Jan 18, 2024
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.

WriteStyle is ignored when used with Target::Pipe
3 participants