Drakulix / simplelog.rs

Simple Logging Facility for Rust
https://docs.rs/simplelog/
Apache License 2.0
424 stars 71 forks source link

why new function return different type? #21

Closed damody closed 6 years ago

damody commented 6 years ago

TermLogger::new return Option<Box> WriteLogger::new return Box

damody commented 6 years ago

and the good init for example

#[macro_use] extern crate log;
extern crate simplelog;
use simplelog::*;
use std::fs::File;
fn main() {
    if term::stderr().is_some()  && term::stdout().is_some() {
        veclog.push(TermLogger::new(LevelFilter::Warn, Config::default()).unwrap());
    }
    let file = File::create(&config.dbg_output_log_path);
    if file.is_ok() {
        veclog.push(WriteLogger::new(LevelFilter::Trace, Config::default(), file.unwrap()));
    }
    CombinedLogger::init(veclog).unwrap();
    error!("Bright red error");
    info!("This only appears in the log file");
    debug!("This level is currently not enabled for any logger");
}
Drakulix commented 6 years ago

Because creating a TermLogger might fail (this should probably be Result<Box<Logger>, _> instead of Option), but creating a WriteLogger cannot.

damody commented 6 years ago
  1. when TermLogger new error by stderr fail, it will panic here https://github.com/Drakulix/simplelog.rs/blob/a7184dc1d5ed609da2553ba0d297c00e718a7cc6/src/loggers/termlog.rs#L106 so we nerver get the Option, it's will panic when get stderr fail.
  2. maybe use filepath replace file, and return file error.
Drakulix commented 6 years ago

term::stderr() will not panic, but instead return None according to the documentation, exactly like term::stdout, which should work out just fine using Option::and_then and Option::map.

damody commented 6 years ago

I use lldb to debug the example program,

#[macro_use] extern crate log;
extern crate simplelog;

use simplelog::*;

use std::fs::File;

fn main() {
    CombinedLogger::init(
        vec![
            TermLogger::new(LevelFilter::Warn, Config::default()).unwrap(),
            WriteLogger::new(LevelFilter::Info, Config::default(), File::create("my_rust_binary.log").unwrap()),
        ]
    ).unwrap();

    error!("Bright red error");
    info!("This only appears in the log file");
    debug!("This level is currently not enabled for any logger");
}

it's die on here https://github.com/Drakulix/simplelog.rs/blob/a7184dc1d5ed609da2553ba0d297c00e718a7cc6/src/loggers/termlog.rs#L106

Drakulix commented 6 years ago

Cannot reproduce with either stable or nightly:

$ rustup update
info: syncing channel updates for 'stable-x86_64-unknown-linux-gnu'
info: syncing channel updates for 'nightly-x86_64-unknown-linux-gnu'
info: checking for self-updates

   stable-x86_64-unknown-linux-gnu unchanged - rustc 1.25.0 (84203cac6 2018-03-25)
   nightly-x86_64-unknown-linux-gnu unchanged - rustc 1.27.0-nightly (8a37c75a3 2018-05-02)

$ rustup run stable cargo run --example usage
    Updating registry `https://github.com/rust-lang/crates.io-index`
   Compiling cfg-if v0.1.2                                                      
   Compiling byteorder v1.2.2
   Compiling libc v0.2.40
   Compiling num-traits v0.2.2
   Compiling log v0.4.1
   Compiling term v0.5.1
   Compiling time v0.1.39
   Compiling num-integer v0.1.36
   Compiling chrono v0.4.2
   Compiling simplelog v0.5.1 (file:///home/drakulix/Projects/simplelog)
    Finished dev [unoptimized + debuginfo] target(s) in 7.66 secs
     Running `target/debug/examples/usage`
12:49:35 [ERROR] Bright red error
$ rustup run nightly cargo run --example usage
   Compiling num-traits v0.2.2
   Compiling libc v0.2.40
   Compiling cfg-if v0.1.2
   Compiling byteorder v1.2.2
   Compiling log v0.4.1
   Compiling term v0.5.1
   Compiling time v0.1.39
   Compiling num-integer v0.1.36
   Compiling chrono v0.4.2
   Compiling simplelog v0.5.1 (file:///home/drakulix/Projects/simplelog)
    Finished dev [unoptimized + debuginfo] target(s) in 10.58 secs
     Running `target/debug/examples/usage`
12:49:54 [ERROR] Bright red error

Please provide more information and reopen if this issue persists.