A few logging improvements #9664
Conversation
|
|
||
| The actual `log_level` is optional to specify. If omitted, all logging will be | ||
| enabled. If specified, the it must be either a numeric in the range of 1-255, or | ||
| it must be one of the strings `debug`, `error`, `info`, or `warn`. |
There was a problem hiding this comment.
Should we mention that (say) log_level == 10 implies log_level == 9 etc (and similar for the symbolic constants)?
(Or that a certain log_level enables all lower levels?)
|
re-pushed with comments, thanks! |
|
Looks good to me! |
|
|
||
| // aux-build:logging_right_crate.rs | ||
| // xfail-fast | ||
| // exec-env:RUST_LOG=logging-right-crate=debug |
There was a problem hiding this comment.
This was actually intentional. Before these changes the program would actually fail because the wrong crate name was used as the top-level name. I suppose that I could make this run-fail, but all it would be doing is inverting the logic.
Right now I ask logging of this crate, and shouldn't get logging of the external crates, but the other way around I'd ask for logging of the external crate and make sure that logging of this crate didn't happen.
There was a problem hiding this comment.
Oh, ok. Probably worth a comment to this effect so that someone trying to work out what the test is doing in a year isn't entirely confused? ;)
There was a problem hiding this comment.
Comment added below.
This makes sure that the top-level crate name is correct when emitting log statements for a monomorphized function in another crate. This happens by tracing the monomorphized ID back to the external source and then using that crate index to get the name of the crate. Closes rust-lang#3046
This adds a large doc-block to the top of the std::logging module explaining how
to use it. This is mostly just making sure that all the information in the
manual's section about logging is also here (in case someone decides to look
into this module first).
This also removes the old console_{on,off} methods. As far as I can tell, the
functions were only used by the compiler, and there's no reason for them to be
used because they're all turned off by default anyway (maybe they were turned on
by default at some point...)
I believe that this is the final nail in the coffin and closes rust-lang#5021
| /// | ||
| /// It is not recommended to call this function directly, rather it should be | ||
| /// invoked through the logging family of macros. | ||
| pub fn log(_level: u32, args: &fmt::Arguments) { |
There was a problem hiding this comment.
Can't we just remove the _level arg?
There was a problem hiding this comment.
I'd rather not completely remove it. In theory we could do things like colorizing log message or something fun like that, but in general it seems like "this might be useful" for a future evolvement of a logging library, even if we're just not using it right now.
My main worry would be that this is a perfectly reasonable function to call directly (via some call chain with format_args! at the top), and it seems appropriate to take a level, and just unfortunate that we don't do anything with it just yet.
There was a problem hiding this comment.
Agreed with @alexcrichton here. Based on the way logging happens, it would be very weird not to have a level argument, It's also strange that it's not being used yet.
This makes some headway on #3309, see commits for details.
This makes some headway on #3309, see commits for details.