From 5e20c0e07deed19aeac651f9c536f96a7c1b262c Mon Sep 17 00:00:00 2001 From: Asuna Date: Mon, 28 Oct 2024 22:36:46 +0800 Subject: [PATCH] Remove `RecordBuilder` --- spdlog/src/formatter/full_formatter.rs | 2 +- spdlog/src/formatter/json_formatter.rs | 15 ++-- spdlog/src/formatter/pattern_formatter/mod.rs | 10 +-- spdlog/src/lib.rs | 23 +++---- spdlog/src/record.rs | 69 +++---------------- spdlog/src/sink/rotating_file_sink.rs | 4 +- 6 files changed, 35 insertions(+), 88 deletions(-) diff --git a/spdlog/src/formatter/full_formatter.rs b/spdlog/src/formatter/full_formatter.rs index c74b9b8f..11b6347f 100644 --- a/spdlog/src/formatter/full_formatter.rs +++ b/spdlog/src/formatter/full_formatter.rs @@ -130,7 +130,7 @@ mod tests { #[test] fn format() { - let record = Record::new(Level::Warn, "test log content"); + let record = Record::new(Level::Warn, "test log content", None, None); let mut buf = StringBuf::new(); let mut ctx = FormatterContext::new(); FullFormatter::new() diff --git a/spdlog/src/formatter/json_formatter.rs b/spdlog/src/formatter/json_formatter.rs index bb106780..dc3f6de0 100644 --- a/spdlog/src/formatter/json_formatter.rs +++ b/spdlog/src/formatter/json_formatter.rs @@ -199,7 +199,7 @@ mod tests { fn should_format_json() { let mut dest = StringBuf::new(); let formatter = JsonFormatter::new(); - let record = Record::builder(Level::Info, "payload").build(); + let record = Record::new(Level::Info, "payload", None, None); let mut ctx = FormatterContext::new(); formatter.format(&record, &mut dest, &mut ctx).unwrap(); @@ -222,9 +222,7 @@ mod tests { fn should_format_json_with_logger_name() { let mut dest = StringBuf::new(); let formatter = JsonFormatter::new(); - let record = Record::builder(Level::Info, "payload") - .logger_name("my-component") - .build(); + let record = Record::new(Level::Info, "payload", None, Some("my-component")); let mut ctx = FormatterContext::new(); formatter.format(&record, &mut dest, &mut ctx).unwrap(); @@ -247,9 +245,12 @@ mod tests { fn should_format_json_with_src_loc() { let mut dest = StringBuf::new(); let formatter = JsonFormatter::new(); - let record = Record::builder(Level::Info, "payload") - .source_location(Some(SourceLocation::__new("module", "file.rs", 1, 2))) - .build(); + let record = Record::new( + Level::Info, + "payload", + Some(SourceLocation::__new("module", "file.rs", 1, 2)), + None, + ); let mut ctx = FormatterContext::new(); formatter.format(&record, &mut dest, &mut ctx).unwrap(); diff --git a/spdlog/src/formatter/pattern_formatter/mod.rs b/spdlog/src/formatter/pattern_formatter/mod.rs index 61b654e2..3f82ddf6 100644 --- a/spdlog/src/formatter/pattern_formatter/mod.rs +++ b/spdlog/src/formatter/pattern_formatter/mod.rs @@ -1210,10 +1210,12 @@ mod tests { #[must_use] fn get_mock_record() -> Record<'static> { - Record::builder(Level::Info, "record_payload") - .logger_name("logger_name") - .source_location(Some(SourceLocation::__new("module", "file", 10, 20))) - .build() + Record::new( + Level::Info, + "record_payload", + Some(SourceLocation::__new("module", "file", 10, 20)), + Some("logger_name"), + ) } fn test_pattern(pattern: P, formatted: T, style_range: Option>) diff --git a/spdlog/src/lib.rs b/spdlog/src/lib.rs index 9f0e68a5..40ba5148 100644 --- a/spdlog/src/lib.rs +++ b/spdlog/src/lib.rs @@ -320,9 +320,10 @@ pub mod prelude { } use std::{ + borrow::Cow, env::{self, VarError}, ffi::OsStr, - panic, + fmt, panic, result::Result as StdResult, }; @@ -783,19 +784,15 @@ pub fn __log( logger: &Logger, level: Level, srcloc: Option, - fmt_args: std::fmt::Arguments, + fmt_args: fmt::Arguments, ) { - // use `Cow` to avoid allocation as much as we can - let payload: std::borrow::Cow = match fmt_args.as_str() { - Some(literal_str) => literal_str.into(), // no format arguments, so it is a `&'static str` - None => fmt_args.to_string().into(), - }; - - let mut builder = Record::builder(level, payload).source_location(srcloc); - if let Some(logger_name) = logger.name() { - builder = builder.logger_name(logger_name); - } - logger.log(&builder.build()); + // Use `Cow` to avoid allocation as much as we can + let payload: Cow = fmt_args + .as_str() + .map(Cow::Borrowed) // No format arguments, so it is a `&'static str` + .unwrap_or_else(|| Cow::Owned(fmt_args.to_string())); + let record = Record::new(level, payload, srcloc, logger.name()); + logger.log(&record); } #[cfg(test)] diff --git a/spdlog/src/record.rs b/spdlog/src/record.rs index c09d03ab..7fe08db0 100644 --- a/spdlog/src/record.rs +++ b/spdlog/src/record.rs @@ -37,30 +37,24 @@ struct RecordInner { impl<'a> Record<'a> { #[must_use] - pub(crate) fn new(level: Level, payload: S) -> Record<'a> - where - S: Into>, - { + pub(crate) fn new( + level: Level, + payload: impl Into>, + srcloc: Option, + logger_name: Option<&'a str>, + ) -> Record<'a> { Record { - logger_name: None, + logger_name: logger_name.map(Cow::Borrowed), payload: payload.into(), inner: Cow::Owned(RecordInner { level, - source_location: None, + source_location: srcloc, time: SystemTime::now(), tid: get_current_tid(), }), } } - #[must_use] - pub(crate) fn builder(level: Level, payload: S) -> RecordBuilder<'a> - where - S: Into>, - { - RecordBuilder::new(level, payload) - } - /// Creates a [`RecordOwned`] that doesn't have lifetimes. #[must_use] pub fn to_owned(&self) -> RecordOwned { @@ -219,53 +213,6 @@ impl RecordOwned { // When adding more getters, also add to `Record` } -#[allow(missing_docs)] -#[derive(Clone, Debug)] -pub(crate) struct RecordBuilder<'a> { - record: Record<'a>, -} - -impl<'a> RecordBuilder<'a> { - /// Constructs a `RecordBuilder`. - /// - /// The default value of [`Record`] is the same as [`Record::new()`]. - /// - /// Typically users should only use it for testing [`Sink`]. - /// - /// [`Sink`]: crate::sink::Sink - #[must_use] - pub(crate) fn new(level: Level, payload: S) -> Self - where - S: Into>, - { - Self { - record: Record::new(level, payload), - } - } - - /// Sets the logger name. - #[must_use] - pub(crate) fn logger_name(mut self, logger_name: &'a str) -> Self { - self.record.logger_name = Some(Cow::Borrowed(logger_name)); - self - } - - /// Sets the source location. - // `Option` in the parameter is for the convenience of passing the result of - // the macro `source_location_current` directly. - #[must_use] - pub(crate) fn source_location(mut self, srcloc: Option) -> Self { - self.record.inner.to_mut().source_location = srcloc; - self - } - - /// Builds a [`Record`]. - #[must_use] - pub(crate) fn build(self) -> Record<'a> { - self.record - } -} - fn get_current_tid() -> u64 { #[cfg(target_os = "linux")] #[must_use] diff --git a/spdlog/src/sink/rotating_file_sink.rs b/spdlog/src/sink/rotating_file_sink.rs index a0926ee3..ea7dc015 100644 --- a/spdlog/src/sink/rotating_file_sink.rs +++ b/spdlog/src/sink/rotating_file_sink.rs @@ -1220,7 +1220,7 @@ mod tests { { let logger = build(true); - let mut record = Record::new(Level::Info, "test log message"); + let mut record = Record::new(Level::Info, "test log message", None, None); let initial_time = record.time(); assert_files_count("hourly", 1); @@ -1279,7 +1279,7 @@ mod tests { }; { - let mut record = Record::new(Level::Info, "test log message"); + let mut record = Record::new(Level::Info, "test log message", None, None); assert_files_count(prefix, 1);