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

Make log target configurable #105

Merged
merged 1 commit into from Apr 18, 2017
Merged

Make log target configurable #105

merged 1 commit into from Apr 18, 2017

Conversation

dbrgn
Copy link
Contributor

@dbrgn dbrgn commented Mar 29, 2017

Right now env_logger always logs to stderr. This might be undesireable in some cases.

This PR adds a way to configure the log target. It can be set to stdout if desired.

Note that writing to stdout is implemented with println! instead of writeln!, so that the output is properly captured in tests. See https://users.rust-lang.org/t/cargo-doesnt-capture-stderr-of-running-tests/9222/.

I did not find out how to properly add a test, but tested it manually. If you want a test to be added, pointers would be welcome.

Right now `env_logger` always logs to stderr. This might be
undesireable in some cases.

This commit adds a way to configure the log target. It can be set to
stdout if desired.

Note that writing to stdout is implemented with `println!` instead of
`writeln!`, so that the output is properly captured in tests.
@dbrgn
Copy link
Contributor Author

dbrgn commented Apr 6, 2017

@alexcrichton @sfackler any chance that someone could take a look at this? It would considerably improve Cargo test output (since right now all test output is intermingled with log messages that aren't properly captured). The change should be quick to review, only around 20 lines of code.

@@ -337,7 +356,12 @@ impl Log for Logger {
}
}

let _ = writeln!(&mut io::stderr(), "{}", (self.format)(record));
match self.target {
LogTarget::Stdout => println!("{}", (self.format)(record)),
Copy link
Member

Choose a reason for hiding this comment

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

Should this be a writeln! to io::stdout instead? There are some differences around capture in the test harness. Not sure which approach would be more expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

writeln! with io::stdout is not being captured by the test harness according to my tests.

@sfackler sfackler merged commit 4195412 into rust-lang:master Apr 18, 2017
@sfackler
Copy link
Member

Thanks!

@dbrgn dbrgn deleted the log-target branch April 18, 2017 09:14
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.

None yet

2 participants