Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Do not colorize output destined for configured logger #664

Merged

Conversation

jasonwebster
Copy link
Contributor

This changes the predicate supplied to the #colorize method to ensure
that if a logger is set, the colorizing ANSI escape codes are not applied.

This definitely appears to have been the intention behind the original
implementation, but the tests didn't reflect how .log_internal was
actually called. In reality, it is always supplied with an out:
argument, not nil. This caused all logger bound output to also be
colorized.

This changes the predicate supplied to the #colorize method to ensure
that if a logger is set, the colorizing ANSI escape codes are not applied.

This definitely appears to have been the intention behind the original
implementation, but the tests didn't reflect how .log_internal was
actually called. In reality, it is always supplied with an `out:`
argument, not nil. This caused all logger bound output to also be
colorized.
@jasonwebster jasonwebster force-pushed the do_not_colorize_logger_logging branch from 1181ae9 to 9d0cd25 Compare July 19, 2018 16:28
@brandur-stripe
Copy link
Contributor

Yes, this looks more right. Thanks!

@brandur-stripe brandur-stripe merged commit b3eb5d3 into stripe:master Jul 19, 2018
@brandur-stripe
Copy link
Contributor

Released as 3.17.2.

@jasonwebster jasonwebster deleted the do_not_colorize_logger_logging branch July 19, 2018 16:58
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants