emabee / flexi_logger

A flexible logger for rust programs that can write to stderr, stdout, and/or to log files
Apache License 2.0
307 stars 50 forks source link

Unexpected behavior about `basename` in `FileSpec` #174

Closed CoolLoong closed 4 hours ago

CoolLoong commented 8 hours ago

1.

pub fn o_basename<S: Into<String>>(mut self, o_basename: Option<S>) -> Self {
        self.basename = o_basename.map_or_else(Self::default_basename, Into::into);
        self
    }

o_basename cannot accept None, because None cannot be into String

2. When using suppress_basename() or basename(""), the log file still starts with a '_' prefix, and the '_' separator should be removed

emabee commented 4 hours ago

Ad 1: Such errors are not possible with rust :-)

I assume you tried writing something like xx.o_basename(None); this fails to compile, with cannot infer type of the type parameter S, because the method accepts any type that implements Into<String>.

You need to help the compiler a bit, e.g. by writing xx.o_basename(Option::<String>::None).

Ad 2: This was reported with issue #153 and should be fixed as of 0.27.4. Are you using an older version of flexi_logger?

There is a test that does this:

        let path = FileSpec::try_from("/a/b/c/d_foo_bar.trc")
            .unwrap()
            .suppress_basename()
            .o_suffix(Some("txt"))
            .o_discriminant(Some("1234"))
            .as_pathbuf(None);
        assert_eq!(path.file_name().unwrap().to_str().unwrap(), "1234.txt");

It is even possible to configure a complete empty FileSpec (which does not make sense for log files, of course, and will fail later):

let path = FileSpec::default()
    .suppress_basename()
    .suppress_timestamp()
    .o_suffix(Option::<String>::None)
    .as_pathbuf(None);
assert!(path.file_name().is_none());
CoolLoong commented 4 hours ago

It seems that the issue hasn't been fixed. The version I'm using is:

flexi_logger = { version = "0.29.0", features = [
    "compress",
    "colors",
    "async",
] }

Here is an example of my configuration

let log_dir = PathBuf::from("logs");
        let file_spec = FileSpec::default()
            .directory(log_dir)
            .suppress_basename()
            .suppress_timestamp()
            .o_suffix(Option::<String>::None)
            .suffix("log");

        let log_to_console_format =
            |write: &mut dyn std::io::Write, now: &mut DeferredNow, record: &Record| {
                let time_prefix = now.format("%H:%M:%S");
                let binding = std::thread::current();
                let thread_name = binding.name().unwrap_or("Unknown thread");
                let level = record.level();
                let file_name = record
                    .file()
                    .map(|file_path| {
                        Path::new(file_path)
                            .file_name()
                            .unwrap_or_else(|| std::ffi::OsStr::new("<unnamed>"))
                            .to_string_lossy()
                            .into_owned()
                    })
                    .unwrap_or_else(|| "<unnamed>".to_string());
                write!(
                    write,
                    "[{}] [{}/{}]: [{}] {}",
                    style(level).paint(time_prefix.to_string()),
                    style(level).paint(thread_name),
                    style(level).paint(level.to_string()),
                    style(level).paint(file_name),
                    &record.args()
                )
            };
        let log_to_file_format =
            |write: &mut dyn std::io::Write, now: &mut DeferredNow, record: &Record| {
                let time_prefix = now.format("%H:%M:%S");
                let binding = std::thread::current();
                let thread_name = binding.name().unwrap_or("Unknown thread");
                let level = record.level();
                let file_name = record
                    .file()
                    .map(|file_path| {
                        Path::new(file_path)
                            .file_name()
                            .unwrap_or_else(|| std::ffi::OsStr::new("<unnamed>"))
                            .to_string_lossy()
                            .into_owned()
                    })
                    .unwrap_or_else(|| "<unnamed>".to_string());
                write!(
                    write,
                    "[{}] [{}/{}]: [{}] {}",
                    time_prefix.to_string(),
                    thread_name,
                    level.to_string(),
                    file_name,
                    &record.args()
                )
            };

        Logger::with(LogSpecification::from(config.log_level))
            .log_to_file(file_spec)
            .rotate(
                flexi_logger::Criterion::Size(10 * 1024 * 1024),
                flexi_logger::Naming::TimestampsDirect,
                flexi_logger::Cleanup::KeepLogAndCompressedFiles(0, 5),
            )
            .set_palette("b1;3;2;4;6".to_string())
            .format_for_files(log_to_file_format)
            .format_for_stderr(log_to_console_format)
            .duplicate_to_stderr(flexi_logger::Duplicate::Info)
            .write_mode(flexi_logger::WriteMode::Async)
            .start()
            .unwrap()

It outputs _r2024-09-24_16-21-18.log, but I expect is r2024-09-24_16-21-18.log