From cd91954ee6dd3909571c85c581badc24ec444df2 Mon Sep 17 00:00:00 2001 From: Stefnotch Date: Tue, 19 Mar 2024 22:09:12 +0100 Subject: [PATCH 01/12] Switch to owo-color --- examples/Cargo.toml | 2 +- .../examples/sloggish/sloggish_collector.rs | 39 ++---- tracing-subscriber/Cargo.toml | 4 +- tracing-subscriber/src/filter/env/builder.rs | 2 +- tracing-subscriber/src/fmt/format/mod.rs | 116 ++++++++++++------ tracing-subscriber/src/fmt/format/pretty.rs | 115 +++++++++-------- 6 files changed, 149 insertions(+), 129 deletions(-) diff --git a/examples/Cargo.toml b/examples/Cargo.toml index f10a324c79..297196ed14 100644 --- a/examples/Cargo.toml +++ b/examples/Cargo.toml @@ -39,7 +39,7 @@ bytes = "1.2.0" argh = "0.1.8" # sloggish example -nu-ansi-term = "0.46.0" +owo-colors = "4.0" humantime = "2.1.0" log = "0.4.17" diff --git a/examples/examples/sloggish/sloggish_collector.rs b/examples/examples/sloggish/sloggish_collector.rs index 32bce14880..504671cbeb 100644 --- a/examples/examples/sloggish/sloggish_collector.rs +++ b/examples/examples/sloggish/sloggish_collector.rs @@ -1,4 +1,4 @@ -use nu_ansi_term::{Color, Style}; +use owo_colors::OwoColorize; use tracing::{ field::{Field, Visit}, Collect, Id, Level, Metadata, @@ -79,13 +79,12 @@ struct ColorLevel<'a>(&'a Level); impl<'a> fmt::Display for ColorLevel<'a> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match *self.0 { - Level::TRACE => Color::Purple.paint("TRACE"), - Level::DEBUG => Color::Blue.paint("DEBUG"), - Level::INFO => Color::Green.paint("INFO "), - Level::WARN => Color::Yellow.paint("WARN "), - Level::ERROR => Color::Red.paint("ERROR"), + Level::TRACE => "TRACE".purple().fmt(f), + Level::DEBUG => "DEBUG".blue().fmt(f), + Level::INFO => "INFO".green().fmt(f), + Level::WARN => "WARN".yellow().fmt(f), + Level::ERROR => "ERROR".red().fmt(f), } - .fmt(f) } } @@ -117,21 +116,9 @@ impl<'a> Visit for Event<'a> { .unwrap(); let name = field.name(); if name == "message" { - write!( - &mut self.stderr, - "{}", - // Have to alloc here due to `nu_ansi_term`'s API... - Style::new().bold().paint(format!("{:?}", value)) - ) - .unwrap(); + write!(&mut self.stderr, "{:?}", value.bold()).unwrap(); } else { - write!( - &mut self.stderr, - "{}: {:?}", - Style::new().bold().paint(name), - value - ) - .unwrap(); + write!(&mut self.stderr, "{}: {:?}", name.bold(), value).unwrap(); } self.comma = true; } @@ -162,16 +149,10 @@ impl SloggishCollector { { let mut kvs = kvs.into_iter(); if let Some((k, v)) = kvs.next() { - write!( - writer, - "{}{}: {}", - leading, - Style::new().bold().paint(k.as_ref()), - v - )?; + write!(writer, "{}{}: {}", leading, k.as_ref().bold(), v)?; } for (k, v) in kvs { - write!(writer, ", {}: {}", Style::new().bold().paint(k.as_ref()), v)?; + write!(writer, ", {}: {}", k.as_ref().bold(), v)?; } Ok(()) } diff --git a/tracing-subscriber/Cargo.toml b/tracing-subscriber/Cargo.toml index 524bf4b482..a1eb698d4b 100644 --- a/tracing-subscriber/Cargo.toml +++ b/tracing-subscriber/Cargo.toml @@ -29,7 +29,7 @@ alloc = ["tracing-core/alloc"] std = ["alloc", "tracing-core/std"] env-filter = ["matchers", "regex", "once_cell", "tracing", "std", "thread_local"] fmt = ["registry", "std"] -ansi = ["fmt", "nu-ansi-term"] +ansi = ["fmt", "owo-colors"] registry = ["sharded-slab", "thread_local", "std"] json = ["tracing-serde", "serde", "serde_json"] @@ -49,7 +49,7 @@ once_cell = { optional = true, version = "1.13.0" } # fmt tracing-log = { path = "../tracing-log", version = "0.2", optional = true, default-features = false, features = ["log-tracer", "std"] } -nu-ansi-term = { version = "0.46.0", optional = true } +owo-colors = { version = "4.0", optional = true } time = { version = "0.3.2", features = ["formatting"], optional = true } # only required by the json feature diff --git a/tracing-subscriber/src/filter/env/builder.rs b/tracing-subscriber/src/filter/env/builder.rs index ebb7b45e72..f16358848e 100644 --- a/tracing-subscriber/src/filter/env/builder.rs +++ b/tracing-subscriber/src/filter/env/builder.rs @@ -223,7 +223,7 @@ impl Builder { let bold = Style::new().bold(); let mut warning = Color::Yellow.paint("warning"); warning.style_ref_mut().is_bold = true; - format!("{}{} {}", warning, bold.paint(":"), bold.paint(msg)) + format!("{}{} {}", warning, ":".style(bold), msg.style(bold)) }; eprintln!("{}", msg); }; diff --git a/tracing-subscriber/src/fmt/format/mod.rs b/tracing-subscriber/src/fmt/format/mod.rs index e19d689486..648aa15345 100644 --- a/tracing-subscriber/src/fmt/format/mod.rs +++ b/tracing-subscriber/src/fmt/format/mod.rs @@ -46,7 +46,7 @@ use tracing_core::{ use tracing_log::NormalizeEvent; #[cfg(feature = "ansi")] -use nu_ansi_term::{Color, Style}; +use owo_colors::{OwoColorize, Style}; #[cfg(feature = "json")] mod json; @@ -977,16 +977,16 @@ where let mut seen = false; for span in scope.from_root() { - write!(writer, "{}", bold.paint(span.metadata().name()))?; + write!(writer, "{}", span.metadata().name().style(bold))?; seen = true; let ext = span.extensions(); if let Some(fields) = &ext.get::>() { if !fields.is_empty() { - write!(writer, "{}{}{}", bold.paint("{"), fields, bold.paint("}"))?; + write!(writer, "{}{}{}", "{".style(bold), fields, "}".style(bold))?; } } - write!(writer, "{}", dimmed.paint(":"))?; + write!(writer, "{}", ":".style(dimmed))?; } if seen { @@ -998,8 +998,8 @@ where write!( writer, "{}{} ", - dimmed.paint(meta.target()), - dimmed.paint(":") + meta.target().style(dimmed), + ":".style(dimmed) )?; } @@ -1014,8 +1014,8 @@ where write!( writer, "{}{}{}", - dimmed.paint(filename), - dimmed.paint(":"), + filename.style(dimmed), + ":".style(dimmed), if line_number.is_some() { "" } else { " " } )?; } @@ -1024,10 +1024,9 @@ where if let Some(line_number) = line_number { write!( writer, - "{}{}:{} ", - dimmed.prefix(), - line_number, - dimmed.suffix() + "{}{} ", + line_number.style(dimmed), + ":".style(dimmed) )?; } @@ -1081,27 +1080,20 @@ where write!( writer, "{}{}", - dimmed.paint(meta.target()), - dimmed.paint(":") + meta.target().style(dimmed), + ":".style(dimmed) )?; } if self.display_filename { if let Some(filename) = meta.file() { - write!(writer, "{}{}", dimmed.paint(filename), dimmed.paint(":"))?; + write!(writer, "{}{}", filename.style(dimmed), ":".style(dimmed))?; } } if self.display_line_number { if let Some(line_number) = meta.line() { - write!( - writer, - "{}{}{}{}", - dimmed.prefix(), - line_number, - dimmed.suffix(), - dimmed.paint(":") - )?; + write!(writer, "{}{}", line_number.style(dimmed), ":".style(dimmed))?; } } @@ -1111,7 +1103,7 @@ where let exts = span.extensions(); if let Some(fields) = exts.get::>() { if !fields.is_empty() { - write!(writer, " {}", dimmed.paint(&fields.fields))?; + write!(writer, " {}", fields.fields.style(dimmed))?; } } } @@ -1223,9 +1215,9 @@ impl<'a> field::Visit for DefaultVisitor<'a> { &format_args!( "{} {}{}{}{}", value, - italic.paint(field.name()), - italic.paint(".sources"), - self.writer.dimmed().paint("="), + field.name().style(italic), + ".sources".style(italic), + "=".style(self.writer.dimmed()), ErrorSourceList(source) ), ) @@ -1248,15 +1240,15 @@ impl<'a> field::Visit for DefaultVisitor<'a> { name if name.starts_with("r#") => write!( self.writer, "{}{}{:?}", - self.writer.italic().paint(&name[2..]), - self.writer.dimmed().paint("="), + (&name[2..]).style(self.writer.italic()), + "=".style(self.writer.dimmed()), value ), name => write!( self.writer, "{}{}{:?}", - self.writer.italic().paint(name), - self.writer.dimmed().paint("="), + name.style(self.writer.italic()), + "=".style(self.writer.dimmed()), value ), }; @@ -1417,11 +1409,11 @@ impl fmt::Display for FmtLevel { { if self.ansi { return match self.level { - Level::TRACE => write!(f, "{}", Color::Purple.paint(F::TRACE_STR)), - Level::DEBUG => write!(f, "{}", Color::Blue.paint(F::DEBUG_STR)), - Level::INFO => write!(f, "{}", Color::Green.paint(F::INFO_STR)), - Level::WARN => write!(f, "{}", Color::Yellow.paint(F::WARN_STR)), - Level::ERROR => write!(f, "{}", Color::Red.paint(F::ERROR_STR)), + Level::TRACE => write!(f, "{}", F::TRACE_STR.purple()), + Level::DEBUG => write!(f, "{}", F::DEBUG_STR.blue()), + Level::INFO => write!(f, "{}", F::INFO_STR.green()), + Level::WARN => write!(f, "{}", F::WARN_STR.yellow()), + Level::ERROR => write!(f, "{}", F::ERROR_STR.red()), }; } } @@ -1648,6 +1640,58 @@ impl Display for TimingDisplay { } } +// Workaround for https://github.com/jam1garner/owo-colors/issues/123 +macro_rules! impl_style_fmt { + ($($trait:path),* $(,)?) => { + $( + impl $trait for StylePrefix<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.style.fmt_prefix(f) + } + } + impl $trait for StyleSuffix<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.style.fmt_suffix(f) + } + } + )* + }; +} + +pub(crate) trait StyleExt { + fn prefix(&self) -> StylePrefix<'_>; + fn suffix(&self) -> StyleSuffix<'_>; +} + +impl StyleExt for Style { + fn prefix(&self) -> StylePrefix<'_> { + StylePrefix { style: self } + } + fn suffix(&self) -> StyleSuffix<'_> { + StyleSuffix { style: self } + } +} + +pub(crate) struct StylePrefix<'a> { + style: &'a Style, +} + +pub(crate) struct StyleSuffix<'a> { + style: &'a Style, +} + +impl_style_fmt! { + fmt::Display, + fmt::Debug, + fmt::UpperHex, + fmt::LowerHex, + fmt::Binary, + fmt::UpperExp, + fmt::LowerExp, + fmt::Octal, + fmt::Pointer, +} + #[cfg(test)] pub(super) mod test { use crate::fmt::{test::MockMakeWriter, time::FormatTime}; diff --git a/tracing-subscriber/src/fmt/format/pretty.rs b/tracing-subscriber/src/fmt/format/pretty.rs index 1a0edd428c..25b123ccc5 100644 --- a/tracing-subscriber/src/fmt/format/pretty.rs +++ b/tracing-subscriber/src/fmt/format/pretty.rs @@ -14,8 +14,6 @@ use tracing_core::{ #[cfg(feature = "tracing-log")] use tracing_log::NormalizeEvent; -use nu_ansi_term::{Color, Style}; - /// An excessively pretty, human-readable event formatter. /// /// Unlike the [`Full`], [`Compact`], and [`Json`] formatters, this is a @@ -143,11 +141,11 @@ impl Default for Pretty { impl Pretty { fn style_for(level: &Level) -> Style { match *level { - Level::TRACE => Style::new().fg(Color::Purple), - Level::DEBUG => Style::new().fg(Color::Blue), - Level::INFO => Style::new().fg(Color::Green), - Level::WARN => Style::new().fg(Color::Yellow), - Level::ERROR => Style::new().fg(Color::Red), + Level::TRACE => Style::new().purple(), + Level::DEBUG => Style::new().blue(), + Level::INFO => Style::new().green(), + Level::WARN => Style::new().yellow(), + Level::ERROR => Style::new().red(), } } @@ -204,13 +202,7 @@ where } else { style }; - write!( - writer, - "{}{}{}:", - target_style.prefix(), - meta.target(), - target_style.infix(style) - )?; + write!(writer, "{}:", meta.target().style(target_style),)?; } let line_number = if self.display_line_number { meta.line() @@ -226,13 +218,7 @@ where self.display_filename, self.format.display_location, ) { - write!( - writer, - "{}{}{}:", - style.prefix(), - line_number, - style.infix(style) - )?; + write!(writer, "{}:", line_number.style(style),)?; } writer.write_char(' ')?; @@ -254,7 +240,7 @@ where self.format.display_location, self.display_filename, ) { - write!(writer, " {} {}", dimmed.paint("at"), file,)?; + write!(writer, " {} {}", "at".style(dimmed), file,)?; if let Some(line) = line_number { write!(writer, ":{}", line)?; @@ -265,7 +251,7 @@ where }; if thread { - write!(writer, "{} ", dimmed.paint("on"))?; + write!(writer, "{} ", "on".style(dimmed))?; let thread = std::thread::current(); if self.display_thread_name { if let Some(name) = thread.name() { @@ -295,16 +281,16 @@ where write!( writer, " {} {}::{}", - dimmed.paint("in"), + "in".style(dimmed), meta.target(), - bold.paint(meta.name()), + meta.name().style(bold), )?; } else { write!( writer, " {} {}", - dimmed.paint("in"), - bold.paint(meta.name()), + "in".style(dimmed), + meta.name().style(bold), )?; } @@ -313,7 +299,7 @@ where .get::>() .expect("Unable to find FormattedFields in extensions; this is a bug"); if !fields.is_empty() { - write!(writer, " {} {}", dimmed.paint("with"), fields)?; + write!(writer, " {} {}", "with".style(dimmed), fields)?; } writer.write_char('\n')?; } @@ -406,14 +392,14 @@ impl<'a> PrettyVisitor<'a> { Self { style, ..self } } - fn write_padded(&mut self, value: &impl fmt::Debug) { - let padding = if self.is_empty { + #[must_use] + fn write_padding(&mut self) -> fmt::Result { + if self.is_empty { self.is_empty = false; - "" + Ok(()) } else { - ", " - }; - self.result = write!(self.writer, "{}{:?}", padding, value); + self.writer.write_str(", ") + } } fn bold(&self) -> Style { @@ -440,16 +426,14 @@ impl<'a> field::Visit for PrettyVisitor<'a> { fn record_error(&mut self, field: &Field, value: &(dyn std::error::Error + 'static)) { if let Some(source) = value.source() { - let bold = self.bold(); + let bold = PrettyVisitor::bold(&self); self.record_debug( field, &format_args!( - "{}, {}{}.sources{}: {}", + "{}, {}.sources: {}", value, - bold.prefix(), - field, - bold.infix(self.style), - ErrorSourceList(source), + field.style(bold), + ErrorSourceList(source).style(self.style), ), ) } else { @@ -461,27 +445,38 @@ impl<'a> field::Visit for PrettyVisitor<'a> { if self.result.is_err() { return; } - let bold = self.bold(); - match field.name() { - "message" => self.write_padded(&format_args!("{}{:?}", self.style.prefix(), value,)), - // Skip fields that are actually log metadata that have already been handled - #[cfg(feature = "tracing-log")] - name if name.starts_with("log.") => self.result = Ok(()), - name if name.starts_with("r#") => self.write_padded(&format_args!( - "{}{}{}: {:?}", - bold.prefix(), - &name[2..], - bold.infix(self.style), - value - )), - name => self.write_padded(&format_args!( - "{}{}{}: {:?}", - bold.prefix(), - name, - bold.infix(self.style), - value - )), + // Would be cleaner with a try { } block, but that's not stable yet. + let mut record_debug_impl = || -> fmt::Result { + let bold = PrettyVisitor::bold(&self); + match field.name() { + "message" => { + self.write_padding()?; + write!(self.writer, "{}{:?}", self.style.prefix(), value) + } + // Skip fields that are actually log metadata that have already been handled + #[cfg(feature = "tracing-log")] + name if name.starts_with("log.") => Ok(()), + name if name.starts_with("r#") => { + self.write_padding()?; + write!( + self.writer, + "{}: {:?}", + (&name[2..]).style(bold), + value.style(self.style) + ) + } + name => { + self.write_padding()?; + write!( + self.writer, + "{}: {:?}", + name.style(bold), + value.style(self.style) + ) + } + } }; + self.result = record_debug_impl(); } } From 361320312170c0c78aa4c5caba86567e2d01695a Mon Sep 17 00:00:00 2001 From: Stefnotch Date: Tue, 19 Mar 2024 22:26:00 +0100 Subject: [PATCH 02/12] Replace nu_ansi_term feature with ansi feature --- tracing-subscriber/src/filter/env/builder.rs | 38 +++++++++----------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/tracing-subscriber/src/filter/env/builder.rs b/tracing-subscriber/src/filter/env/builder.rs index f16358848e..f1fc98deb1 100644 --- a/tracing-subscriber/src/filter/env/builder.rs +++ b/tracing-subscriber/src/filter/env/builder.rs @@ -210,45 +210,39 @@ impl Builder { } if !disabled.is_empty() { - #[cfg(feature = "nu_ansi_term")] - use nu_ansi_term::{Color, Style}; + #[cfg(feature = "ansi")] + use owo_colors::{OwoColorize, XtermColors}; // NOTE: We can't use a configured `MakeWriter` because the EnvFilter // has no knowledge of any underlying subscriber or collector, which // may or may not use a `MakeWriter`. let warn = |msg: &str| { - #[cfg(not(feature = "nu_ansi_term"))] + #[cfg(not(feature = "ansi"))] let msg = format!("warning: {}", msg); - #[cfg(feature = "nu_ansi_term")] - let msg = { - let bold = Style::new().bold(); - let mut warning = Color::Yellow.paint("warning"); - warning.style_ref_mut().is_bold = true; - format!("{}{} {}", warning, ":".style(bold), msg.style(bold)) - }; + #[cfg(feature = "ansi")] + let msg = { format!("{}{} {}", "warning".yellow().bold(), ":".bold(), msg.bold()) }; eprintln!("{}", msg); }; let ctx_prefixed = |prefix: &str, msg: &str| { - #[cfg(not(feature = "nu_ansi_term"))] + #[cfg(not(feature = "ansi"))] let msg = format!("{} {}", prefix, msg); - #[cfg(feature = "nu_ansi_term")] + #[cfg(feature = "ansi")] let msg = { - let mut equal = Color::Fixed(21).paint("="); // dark blue - equal.style_ref_mut().is_bold = true; - format!(" {} {} {}", equal, Style::new().bold().paint(prefix), msg) + format!( + " {} {} {}", + "=".color(XtermColors::Blue).bold(), + prefix.bold(), + msg + ) }; eprintln!("{}", msg); }; let ctx_help = |msg| ctx_prefixed("help:", msg); let ctx_note = |msg| ctx_prefixed("note:", msg); let ctx = |msg: &str| { - #[cfg(not(feature = "nu_ansi_term"))] + #[cfg(not(feature = "ansi"))] let msg = format!("note: {}", msg); - #[cfg(feature = "nu_ansi_term")] - let msg = { - let mut pipe = Color::Fixed(21).paint("|"); - pipe.style_ref_mut().is_bold = true; - format!(" {} {}", pipe, msg) - }; + #[cfg(feature = "ansi")] + let msg = { format!(" {} {}", "|".color(XtermColors::Blue).bold(), msg) }; eprintln!("{}", msg); }; warn("some trace filter directives would enable traces that are disabled statically"); From 916f3cf77532ca107cc962908b7f5ed5551de1e3 Mon Sep 17 00:00:00 2001 From: Stefnotch Date: Tue, 19 Mar 2024 22:41:59 +0100 Subject: [PATCH 03/12] Update unit test for owo-color --- tracing-subscriber/src/fmt/format/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tracing-subscriber/src/fmt/format/mod.rs b/tracing-subscriber/src/fmt/format/mod.rs index 648aa15345..be54237aae 100644 --- a/tracing-subscriber/src/fmt/format/mod.rs +++ b/tracing-subscriber/src/fmt/format/mod.rs @@ -1732,7 +1732,7 @@ pub(super) mod test { #[cfg(feature = "ansi")] #[test] fn with_ansi_true() { - let expected = "\u{1b}[2mfake time\u{1b}[0m \u{1b}[32m INFO\u{1b}[0m \u{1b}[2mtracing_subscriber::fmt::format::test\u{1b}[0m\u{1b}[2m:\u{1b}[0m hello\n"; + let expected = "\u{1b}[2mfake time\u{1b}[0m \u{1b}[32m INFO\u{1b}[39m \u{1b}[2mtracing_subscriber::fmt::format::test\u{1b}[0m\u{1b}[2m:\u{1b}[0m hello\n"; assert_info_hello_ansi(true, expected); } From 04b32306e4a7df1ba62e2461aaa86a1bee0f19e5 Mon Sep 17 00:00:00 2001 From: Stefnotch Date: Wed, 20 Mar 2024 11:36:20 +0100 Subject: [PATCH 04/12] Fixes some minor styling --- tracing-subscriber/src/fmt/format/pretty.rs | 32 ++++++++++++--------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/tracing-subscriber/src/fmt/format/pretty.rs b/tracing-subscriber/src/fmt/format/pretty.rs index 25b123ccc5..3106e856d4 100644 --- a/tracing-subscriber/src/fmt/format/pretty.rs +++ b/tracing-subscriber/src/fmt/format/pretty.rs @@ -202,7 +202,12 @@ where } else { style }; - write!(writer, "{}:", meta.target().style(target_style),)?; + write!( + writer, + "{}{}", + meta.target().style(target_style), + ":".style(style) + )?; } let line_number = if self.display_line_number { meta.line() @@ -218,7 +223,7 @@ where self.display_filename, self.format.display_location, ) { - write!(writer, "{}:", line_number.style(style),)?; + write!(writer, "{}{}", line_number.style(style), ":".style(style))?; } writer.write_char(' ')?; @@ -228,11 +233,7 @@ where v.finish()?; writer.write_char('\n')?; - let dimmed = if writer.has_ansi_escapes() { - Style::new().dimmed().italic() - } else { - Style::new() - }; + let dimmed = writer.dimmed(); let thread = self.display_thread_name || self.display_thread_id; if let (Some(file), true, true) = ( @@ -389,7 +390,11 @@ impl<'a> PrettyVisitor<'a> { } pub(crate) fn with_style(self, style: Style) -> Self { - Self { style, ..self } + if self.writer.has_ansi_escapes() { + Self { style, ..self } + } else { + self + } } #[must_use] @@ -402,11 +407,11 @@ impl<'a> PrettyVisitor<'a> { } } - fn bold(&self) -> Style { + fn with_bold(&self) -> Style { if self.writer.has_ansi_escapes() { self.style.bold() } else { - Style::new() + self.style } } } @@ -426,13 +431,14 @@ impl<'a> field::Visit for PrettyVisitor<'a> { fn record_error(&mut self, field: &Field, value: &(dyn std::error::Error + 'static)) { if let Some(source) = value.source() { - let bold = PrettyVisitor::bold(&self); + let bold = self.with_bold(); self.record_debug( field, &format_args!( - "{}, {}.sources: {}", + "{}, {}{}: {}", value, field.style(bold), + ".sources".style(bold), ErrorSourceList(source).style(self.style), ), ) @@ -447,7 +453,7 @@ impl<'a> field::Visit for PrettyVisitor<'a> { } // Would be cleaner with a try { } block, but that's not stable yet. let mut record_debug_impl = || -> fmt::Result { - let bold = PrettyVisitor::bold(&self); + let bold = self.with_bold(); match field.name() { "message" => { self.write_padding()?; From 51758ee3e5f83e7857f32e0ae3f60fb2ae18735a Mon Sep 17 00:00:00 2001 From: Stefnotch Date: Wed, 20 Mar 2024 13:12:17 +0100 Subject: [PATCH 05/12] Switch back to .paint() API --- tracing-subscriber/src/fmt/format/mod.rs | 206 +++++++++----------- tracing-subscriber/src/fmt/format/pretty.rs | 130 ++++++------ 2 files changed, 165 insertions(+), 171 deletions(-) diff --git a/tracing-subscriber/src/fmt/format/mod.rs b/tracing-subscriber/src/fmt/format/mod.rs index be54237aae..64c6d2e216 100644 --- a/tracing-subscriber/src/fmt/format/mod.rs +++ b/tracing-subscriber/src/fmt/format/mod.rs @@ -46,7 +46,7 @@ use tracing_core::{ use tracing_log::NormalizeEvent; #[cfg(feature = "ansi")] -use owo_colors::{OwoColorize, Style}; +use owo_colors::{Style, Styled}; #[cfg(feature = "json")] mod json; @@ -528,7 +528,7 @@ impl<'writer> Writer<'writer> { self.is_ansi } - pub(in crate::fmt::format) fn bold(&self) -> Style { + pub(in crate::fmt::format) fn bold_style(&self) -> Style { #[cfg(feature = "ansi")] { if self.is_ansi { @@ -539,7 +539,7 @@ impl<'writer> Writer<'writer> { Style::new() } - pub(in crate::fmt::format) fn dimmed(&self) -> Style { + pub(in crate::fmt::format) fn dimmed_style(&self) -> Style { #[cfg(feature = "ansi")] { if self.is_ansi { @@ -550,7 +550,7 @@ impl<'writer> Writer<'writer> { Style::new() } - pub(in crate::fmt::format) fn italic(&self) -> Style { + pub(in crate::fmt::format) fn italic_style(&self) -> Style { #[cfg(feature = "ansi")] { if self.is_ansi { @@ -851,32 +851,8 @@ impl Format { return Ok(()); } - // If ANSI color codes are enabled, format the timestamp with ANSI - // colors. - #[cfg(feature = "ansi")] - { - if writer.has_ansi_escapes() { - let style = Style::new().dimmed(); - write!(writer, "{}", style.prefix())?; - - // If getting the timestamp failed, don't bail --- only bail on - // formatting errors. - if self.timer.format_time(writer).is_err() { - writer.write_str("")?; - } - - write!(writer, "{} ", style.suffix())?; - return Ok(()); - } - } - - // Otherwise, just format the timestamp without ANSI formatting. - // If getting the timestamp failed, don't bail --- only bail on - // formatting errors. - if self.timer.format_time(writer).is_err() { - writer.write_str("")?; - } - writer.write_char(' ') + let dimmed = writer.dimmed_style(); + write!(writer, "{} ", dimmed.paint(FormatTimeDisplay(&self.timer))) } } @@ -969,24 +945,24 @@ where write!(writer, "{:0>2?} ", std::thread::current().id())?; } - let dimmed = writer.dimmed(); + let dimmed = writer.dimmed_style(); if let Some(scope) = ctx.event_scope() { - let bold = writer.bold(); + let bold = writer.bold_style(); let mut seen = false; for span in scope.from_root() { - write!(writer, "{}", span.metadata().name().style(bold))?; + write!(writer, "{}", bold.paint(span.metadata().name()))?; seen = true; let ext = span.extensions(); if let Some(fields) = &ext.get::>() { if !fields.is_empty() { - write!(writer, "{}{}{}", "{".style(bold), fields, "}".style(bold))?; + write!(writer, "{}{}{}", bold.paint("{"), fields, bold.paint("}"))?; } } - write!(writer, "{}", ":".style(dimmed))?; + write!(writer, "{}", dimmed.paint(":"))?; } if seen { @@ -998,8 +974,8 @@ where write!( writer, "{}{} ", - meta.target().style(dimmed), - ":".style(dimmed) + dimmed.paint(meta.target()), + dimmed.paint(":") )?; } @@ -1014,8 +990,8 @@ where write!( writer, "{}{}{}", - filename.style(dimmed), - ":".style(dimmed), + dimmed.paint(filename), + dimmed.paint(":"), if line_number.is_some() { "" } else { " " } )?; } @@ -1025,8 +1001,8 @@ where write!( writer, "{}{} ", - line_number.style(dimmed), - ":".style(dimmed) + dimmed.paint(line_number), + dimmed.paint(":") )?; } @@ -1075,25 +1051,25 @@ where write!(writer, "{:0>2?} ", std::thread::current().id())?; } - let dimmed = writer.dimmed(); + let dimmed = writer.dimmed_style(); if self.display_target { write!( writer, "{}{}", - meta.target().style(dimmed), - ":".style(dimmed) + dimmed.paint(meta.target()), + dimmed.paint(":") )?; } if self.display_filename { if let Some(filename) = meta.file() { - write!(writer, "{}{}", filename.style(dimmed), ":".style(dimmed))?; + write!(writer, "{}{}", dimmed.paint(filename), dimmed.paint(":"))?; } } if self.display_line_number { if let Some(line_number) = meta.line() { - write!(writer, "{}{}", line_number.style(dimmed), ":".style(dimmed))?; + write!(writer, "{}{}", dimmed.paint(line_number), dimmed.paint(":"))?; } } @@ -1103,7 +1079,7 @@ where let exts = span.extensions(); if let Some(fields) = exts.get::>() { if !fields.is_empty() { - write!(writer, " {}", fields.fields.style(dimmed))?; + write!(writer, " {}", dimmed.paint(&fields.fields))?; } } } @@ -1209,15 +1185,15 @@ impl<'a> field::Visit for DefaultVisitor<'a> { fn record_error(&mut self, field: &Field, value: &(dyn std::error::Error + 'static)) { if let Some(source) = value.source() { - let italic = self.writer.italic(); + let italic = self.writer.italic_style(); self.record_debug( field, &format_args!( "{} {}{}{}{}", value, - field.name().style(italic), - ".sources".style(italic), - "=".style(self.writer.dimmed()), + italic.paint(field.name()), + italic.paint(".sources"), + self.writer.dimmed_style().paint("="), ErrorSourceList(source) ), ) @@ -1240,15 +1216,15 @@ impl<'a> field::Visit for DefaultVisitor<'a> { name if name.starts_with("r#") => write!( self.writer, "{}{}{:?}", - (&name[2..]).style(self.writer.italic()), - "=".style(self.writer.dimmed()), + self.writer.italic_style().paint(&name[2..]), + self.writer.dimmed_style().paint("="), value ), name => write!( self.writer, "{}{}{:?}", - name.style(self.writer.italic()), - "=".style(self.writer.dimmed()), + self.writer.italic_style().paint(name), + self.writer.dimmed_style().paint("="), value ), }; @@ -1282,7 +1258,12 @@ impl<'a> Display for ErrorSourceList<'a> { } } +trait StylePainter { + fn paint(&self, d: T) -> Styled; +} + #[cfg(not(feature = "ansi"))] +#[derive(Copy, Clone)] struct Style; #[cfg(not(feature = "ansi"))] @@ -1291,16 +1272,59 @@ impl Style { Style } - fn paint(&self, d: impl fmt::Display) -> impl fmt::Display { - d + fn bold(self) -> Self { + self + } + + fn dimmed(self) -> Self { + self } - fn prefix(&self) -> impl fmt::Display { - "" + fn italic(self) -> Self { + self } +} +#[cfg(not(feature = "ansi"))] +impl StylePainter for Style { + fn paint(&self, d: T) -> Styled { + Styled { target: d } + } +} + +#[cfg(not(feature = "ansi"))] +struct Styled { + target: T, +} + +#[cfg(not(feature = "ansi"))] +macro_rules! impl_fmt { + ($($trait:path),* $(,)?) => { + $( + impl $trait for Styled { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + ::fmt(&self.target, f) + } + } + )* + }; +} +#[cfg(not(feature = "ansi"))] +impl_fmt! { + fmt::Display, + fmt::Debug, + fmt::UpperHex, + fmt::LowerHex, + fmt::Binary, + fmt::UpperExp, + fmt::LowerExp, + fmt::Octal, + fmt::Pointer, +} - fn suffix(&self) -> impl fmt::Display { - "" +#[cfg(feature = "ansi")] +impl StylePainter for Style { + fn paint(&self, d: T) -> Styled { + self.style(d) } } @@ -1407,6 +1431,7 @@ impl fmt::Display for FmtLevel { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { #[cfg(feature = "ansi")] { + use owo_colors::OwoColorize; if self.ansi { return match self.level { Level::TRACE => write!(f, "{}", F::TRACE_STR.purple()), @@ -1640,58 +1665,19 @@ impl Display for TimingDisplay { } } -// Workaround for https://github.com/jam1garner/owo-colors/issues/123 -macro_rules! impl_style_fmt { - ($($trait:path),* $(,)?) => { - $( - impl $trait for StylePrefix<'_> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.style.fmt_prefix(f) - } - } - impl $trait for StyleSuffix<'_> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.style.fmt_suffix(f) - } - } - )* - }; -} - -pub(crate) trait StyleExt { - fn prefix(&self) -> StylePrefix<'_>; - fn suffix(&self) -> StyleSuffix<'_>; -} - -impl StyleExt for Style { - fn prefix(&self) -> StylePrefix<'_> { - StylePrefix { style: self } - } - fn suffix(&self) -> StyleSuffix<'_> { - StyleSuffix { style: self } +struct FormatTimeDisplay(T); +impl fmt::Display for FormatTimeDisplay { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + // If getting the timestamp failed, don't bail --- only bail on + // formatting errors. + if self.0.format_time(&mut Writer::new(f)).is_err() { + f.write_str("") + } else { + Ok(()) + } } } -pub(crate) struct StylePrefix<'a> { - style: &'a Style, -} - -pub(crate) struct StyleSuffix<'a> { - style: &'a Style, -} - -impl_style_fmt! { - fmt::Display, - fmt::Debug, - fmt::UpperHex, - fmt::LowerHex, - fmt::Binary, - fmt::UpperExp, - fmt::LowerExp, - fmt::Octal, - fmt::Pointer, -} - #[cfg(test)] pub(super) mod test { use crate::fmt::{test::MockMakeWriter, time::FormatTime}; diff --git a/tracing-subscriber/src/fmt/format/pretty.rs b/tracing-subscriber/src/fmt/format/pretty.rs index 3106e856d4..a3a7c3cbfe 100644 --- a/tracing-subscriber/src/fmt/format/pretty.rs +++ b/tracing-subscriber/src/fmt/format/pretty.rs @@ -205,8 +205,8 @@ where write!( writer, "{}{}", - meta.target().style(target_style), - ":".style(style) + target_style.paint(meta.target()), + style.paint(":") )?; } let line_number = if self.display_line_number { @@ -223,7 +223,7 @@ where self.display_filename, self.format.display_location, ) { - write!(writer, "{}{}", line_number.style(style), ":".style(style))?; + write!(writer, "{}{}", style.paint(line_number), style.paint(":"))?; } writer.write_char(' ')?; @@ -233,7 +233,7 @@ where v.finish()?; writer.write_char('\n')?; - let dimmed = writer.dimmed(); + let dimmed = writer.dimmed_style(); let thread = self.display_thread_name || self.display_thread_id; if let (Some(file), true, true) = ( @@ -241,7 +241,7 @@ where self.format.display_location, self.display_filename, ) { - write!(writer, " {} {}", "at".style(dimmed), file,)?; + write!(writer, " {} {}", dimmed.paint("at"), file,)?; if let Some(line) = line_number { write!(writer, ":{}", line)?; @@ -252,7 +252,7 @@ where }; if thread { - write!(writer, "{} ", "on".style(dimmed))?; + write!(writer, "{} ", dimmed.paint("on"))?; let thread = std::thread::current(); if self.display_thread_name { if let Some(name) = thread.name() { @@ -268,7 +268,7 @@ where writer.write_char('\n')?; } - let bold = writer.bold(); + let bold = writer.bold_style(); let span = event .parent() .and_then(|id| ctx.span(id)) @@ -282,16 +282,16 @@ where write!( writer, " {} {}::{}", - "in".style(dimmed), + dimmed.paint("in"), meta.target(), - meta.name().style(bold), + bold.paint(meta.name()), )?; } else { write!( writer, " {} {}", - "in".style(dimmed), - meta.name().style(bold), + dimmed.paint("in"), + bold.paint(meta.name()), )?; } @@ -300,7 +300,7 @@ where .get::>() .expect("Unable to find FormattedFields in extensions; this is a bug"); if !fields.is_empty() { - write!(writer, " {} {}", "with".style(dimmed), fields)?; + write!(writer, " {} {}", dimmed.paint("with"), fields)?; } writer.write_char('\n')?; } @@ -403,17 +403,51 @@ impl<'a> PrettyVisitor<'a> { self.is_empty = false; Ok(()) } else { - self.writer.write_str(", ") + write!(self.writer, "{}", self.style.paint(", ")) } } - fn with_bold(&self) -> Style { + fn bold_style(&self) -> Style { if self.writer.has_ansi_escapes() { self.style.bold() } else { self.style } } + + #[must_use] + fn record_debug_impl(&mut self, field: &Field, styled_value: &dyn fmt::Debug) -> fmt::Result { + let bold = self.bold_style(); + match field.name() { + "message" => { + self.write_padding()?; + write!(self.writer, "{:?}", styled_value) + } + // Skip fields that are actually log metadata that have already been handled + #[cfg(feature = "tracing-log")] + name if name.starts_with("log.") => Ok(()), + name if name.starts_with("r#") => { + self.write_padding()?; + write!( + self.writer, + "{}{} {:?}", + bold.paint(&name[2..]), + bold.paint(":"), + styled_value + ) + } + name => { + self.write_padding()?; + write!( + self.writer, + "{}{} {:?}", + bold.paint(name), + bold.paint(":"), + styled_value + ) + } + } + } } impl<'a> field::Visit for PrettyVisitor<'a> { @@ -422,28 +456,34 @@ impl<'a> field::Visit for PrettyVisitor<'a> { return; } - if field.name() == "message" { - self.record_debug(field, &format_args!("{}", value)) + self.result = if field.name() == "message" { + self.record_debug_impl(field, &format_args!("{}", self.style.paint(value))) } else { - self.record_debug(field, &value) + self.record_debug_impl(field, &self.style.paint(value)) } } fn record_error(&mut self, field: &Field, value: &(dyn std::error::Error + 'static)) { - if let Some(source) = value.source() { - let bold = self.with_bold(); - self.record_debug( + if self.result.is_err() { + return; + } + + self.result = if let Some(source) = value.source() { + let bold: Style = self.bold_style(); + self.record_debug_impl( field, &format_args!( - "{}, {}{}: {}", - value, - field.style(bold), - ".sources".style(bold), - ErrorSourceList(source).style(self.style), + "{}{} {}{}{} {}", + self.style.paint(value), + self.style.paint(","), + bold.paint(field), + bold.paint(".sources"), + self.style.paint(":"), + self.style.paint(ErrorSourceList(source)) ), ) } else { - self.record_debug(field, &format_args!("{}", value)) + self.record_debug_impl(field, &format_args!("{}", self.style.paint(value))) } } @@ -451,45 +491,13 @@ impl<'a> field::Visit for PrettyVisitor<'a> { if self.result.is_err() { return; } - // Would be cleaner with a try { } block, but that's not stable yet. - let mut record_debug_impl = || -> fmt::Result { - let bold = self.with_bold(); - match field.name() { - "message" => { - self.write_padding()?; - write!(self.writer, "{}{:?}", self.style.prefix(), value) - } - // Skip fields that are actually log metadata that have already been handled - #[cfg(feature = "tracing-log")] - name if name.starts_with("log.") => Ok(()), - name if name.starts_with("r#") => { - self.write_padding()?; - write!( - self.writer, - "{}: {:?}", - (&name[2..]).style(bold), - value.style(self.style) - ) - } - name => { - self.write_padding()?; - write!( - self.writer, - "{}: {:?}", - name.style(bold), - value.style(self.style) - ) - } - } - }; - self.result = record_debug_impl(); + self.result = self.record_debug_impl(field, &self.style.paint(value)); } } impl<'a> VisitOutput for PrettyVisitor<'a> { - fn finish(mut self) -> fmt::Result { - write!(&mut self.writer, "{}", self.style.suffix())?; - self.result + fn finish(self) -> fmt::Result { + Ok(()) } } From 70aef56d8231be4517bc2e69176baa5eeefbcae8 Mon Sep 17 00:00:00 2001 From: Stefnotch Date: Wed, 20 Mar 2024 13:26:07 +0100 Subject: [PATCH 06/12] Fix dimmed-italic style usage --- tracing-subscriber/src/fmt/format/pretty.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/tracing-subscriber/src/fmt/format/pretty.rs b/tracing-subscriber/src/fmt/format/pretty.rs index a3a7c3cbfe..67d01bd993 100644 --- a/tracing-subscriber/src/fmt/format/pretty.rs +++ b/tracing-subscriber/src/fmt/format/pretty.rs @@ -233,7 +233,11 @@ where v.finish()?; writer.write_char('\n')?; - let dimmed = writer.dimmed_style(); + let dimmed_italic = if writer.has_ansi_escapes() { + Style::new().dimmed().italic() + } else { + Style::new() + }; let thread = self.display_thread_name || self.display_thread_id; if let (Some(file), true, true) = ( @@ -241,7 +245,7 @@ where self.format.display_location, self.display_filename, ) { - write!(writer, " {} {}", dimmed.paint("at"), file,)?; + write!(writer, " {} {}", dimmed_italic.paint("at"), file,)?; if let Some(line) = line_number { write!(writer, ":{}", line)?; @@ -252,7 +256,7 @@ where }; if thread { - write!(writer, "{} ", dimmed.paint("on"))?; + write!(writer, "{} ", dimmed_italic.paint("on"))?; let thread = std::thread::current(); if self.display_thread_name { if let Some(name) = thread.name() { @@ -282,7 +286,7 @@ where write!( writer, " {} {}::{}", - dimmed.paint("in"), + dimmed_italic.paint("in"), meta.target(), bold.paint(meta.name()), )?; @@ -290,7 +294,7 @@ where write!( writer, " {} {}", - dimmed.paint("in"), + dimmed_italic.paint("in"), bold.paint(meta.name()), )?; } @@ -300,7 +304,7 @@ where .get::>() .expect("Unable to find FormattedFields in extensions; this is a bug"); if !fields.is_empty() { - write!(writer, " {} {}", dimmed.paint("with"), fields)?; + write!(writer, " {} {}", dimmed_italic.paint("with"), fields)?; } writer.write_char('\n')?; } From f3ee65c455aa9cbcf6fd8b1402961473e398b450 Mon Sep 17 00:00:00 2001 From: Stefnotch Date: Fri, 22 Mar 2024 09:33:41 +0100 Subject: [PATCH 07/12] Undo a rename to keep the changes small --- tracing-subscriber/src/fmt/format/mod.rs | 29 +++++++++++---------- tracing-subscriber/src/fmt/format/pretty.rs | 2 +- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/tracing-subscriber/src/fmt/format/mod.rs b/tracing-subscriber/src/fmt/format/mod.rs index 64c6d2e216..6fafe89479 100644 --- a/tracing-subscriber/src/fmt/format/mod.rs +++ b/tracing-subscriber/src/fmt/format/mod.rs @@ -103,7 +103,7 @@ use fmt::{Debug, Display}; /// does not support ANSI escape codes (such as a log file), and they should /// not be emitted. /// -/// Crates like [`nu_ansi_term`] and [`owo-colors`] can be used to add ANSI +/// Crates like [`owo-colors`] and [`nu_ansi_term`] can be used to add ANSI /// escape codes to formatted output. /// /// * The actual [`Event`] to be formatted. @@ -528,7 +528,7 @@ impl<'writer> Writer<'writer> { self.is_ansi } - pub(in crate::fmt::format) fn bold_style(&self) -> Style { + pub(in crate::fmt::format) fn bold(&self) -> Style { #[cfg(feature = "ansi")] { if self.is_ansi { @@ -539,7 +539,7 @@ impl<'writer> Writer<'writer> { Style::new() } - pub(in crate::fmt::format) fn dimmed_style(&self) -> Style { + pub(in crate::fmt::format) fn dimmed(&self) -> Style { #[cfg(feature = "ansi")] { if self.is_ansi { @@ -550,7 +550,7 @@ impl<'writer> Writer<'writer> { Style::new() } - pub(in crate::fmt::format) fn italic_style(&self) -> Style { + pub(in crate::fmt::format) fn italic(&self) -> Style { #[cfg(feature = "ansi")] { if self.is_ansi { @@ -851,7 +851,7 @@ impl Format { return Ok(()); } - let dimmed = writer.dimmed_style(); + let dimmed = writer.dimmed(); write!(writer, "{} ", dimmed.paint(FormatTimeDisplay(&self.timer))) } } @@ -945,10 +945,10 @@ where write!(writer, "{:0>2?} ", std::thread::current().id())?; } - let dimmed = writer.dimmed_style(); + let dimmed = writer.dimmed(); if let Some(scope) = ctx.event_scope() { - let bold = writer.bold_style(); + let bold = writer.bold(); let mut seen = false; @@ -1051,7 +1051,7 @@ where write!(writer, "{:0>2?} ", std::thread::current().id())?; } - let dimmed = writer.dimmed_style(); + let dimmed = writer.dimmed(); if self.display_target { write!( writer, @@ -1185,7 +1185,7 @@ impl<'a> field::Visit for DefaultVisitor<'a> { fn record_error(&mut self, field: &Field, value: &(dyn std::error::Error + 'static)) { if let Some(source) = value.source() { - let italic = self.writer.italic_style(); + let italic = self.writer.italic(); self.record_debug( field, &format_args!( @@ -1193,7 +1193,7 @@ impl<'a> field::Visit for DefaultVisitor<'a> { value, italic.paint(field.name()), italic.paint(".sources"), - self.writer.dimmed_style().paint("="), + self.writer.dimmed().paint("="), ErrorSourceList(source) ), ) @@ -1216,15 +1216,15 @@ impl<'a> field::Visit for DefaultVisitor<'a> { name if name.starts_with("r#") => write!( self.writer, "{}{}{:?}", - self.writer.italic_style().paint(&name[2..]), - self.writer.dimmed_style().paint("="), + self.writer.italic().paint(&name[2..]), + self.writer.dimmed().paint("="), value ), name => write!( self.writer, "{}{}{:?}", - self.writer.italic_style().paint(name), - self.writer.dimmed_style().paint("="), + self.writer.italic().paint(name), + self.writer.dimmed().paint("="), value ), }; @@ -1431,6 +1431,7 @@ impl fmt::Display for FmtLevel { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { #[cfg(feature = "ansi")] { + // Be careful about importing owo_colors::OwoColorize in a too large scope, since it adds methods to every type. use owo_colors::OwoColorize; if self.ansi { return match self.level { diff --git a/tracing-subscriber/src/fmt/format/pretty.rs b/tracing-subscriber/src/fmt/format/pretty.rs index 67d01bd993..703c21bf3b 100644 --- a/tracing-subscriber/src/fmt/format/pretty.rs +++ b/tracing-subscriber/src/fmt/format/pretty.rs @@ -272,7 +272,7 @@ where writer.write_char('\n')?; } - let bold = writer.bold_style(); + let bold = writer.bold(); let span = event .parent() .and_then(|id| ctx.span(id)) From 7f990f2b19580340f57382e8dc31578a7f16ff57 Mon Sep 17 00:00:00 2001 From: Stefnotch Date: Tue, 26 Mar 2024 11:32:45 +0100 Subject: [PATCH 08/12] Return result in PrettyVisitor.finish --- tracing-subscriber/src/fmt/format/pretty.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tracing-subscriber/src/fmt/format/pretty.rs b/tracing-subscriber/src/fmt/format/pretty.rs index 703c21bf3b..aaa3e5947a 100644 --- a/tracing-subscriber/src/fmt/format/pretty.rs +++ b/tracing-subscriber/src/fmt/format/pretty.rs @@ -501,7 +501,7 @@ impl<'a> field::Visit for PrettyVisitor<'a> { impl<'a> VisitOutput for PrettyVisitor<'a> { fn finish(self) -> fmt::Result { - Ok(()) + self.result } } From 91a2622bbfe9d3052314028ea89927787b01918c Mon Sep 17 00:00:00 2001 From: Stefnotch Date: Tue, 26 Mar 2024 11:34:10 +0100 Subject: [PATCH 09/12] Re-add the extra space --- examples/examples/sloggish/sloggish_collector.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/examples/examples/sloggish/sloggish_collector.rs b/examples/examples/sloggish/sloggish_collector.rs index 504671cbeb..0cabacacfa 100644 --- a/examples/examples/sloggish/sloggish_collector.rs +++ b/examples/examples/sloggish/sloggish_collector.rs @@ -79,10 +79,11 @@ struct ColorLevel<'a>(&'a Level); impl<'a> fmt::Display for ColorLevel<'a> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match *self.0 { + /* Extra space to keep the width consistent */ Level::TRACE => "TRACE".purple().fmt(f), Level::DEBUG => "DEBUG".blue().fmt(f), - Level::INFO => "INFO".green().fmt(f), - Level::WARN => "WARN".yellow().fmt(f), + Level::INFO => "INFO ".green().fmt(f), + Level::WARN => "WARN ".yellow().fmt(f), Level::ERROR => "ERROR".red().fmt(f), } } From dd1c47b382452c0671985f5a56eab3f62e18c5fb Mon Sep 17 00:00:00 2001 From: Stefnotch Date: Tue, 26 Mar 2024 11:53:05 +0100 Subject: [PATCH 10/12] Fix usage of writer.has_ansi_escapes in pretty printer --- tracing-subscriber/src/fmt/format/pretty.rs | 33 ++++++++++++--------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/tracing-subscriber/src/fmt/format/pretty.rs b/tracing-subscriber/src/fmt/format/pretty.rs index aaa3e5947a..909bc7762a 100644 --- a/tracing-subscriber/src/fmt/format/pretty.rs +++ b/tracing-subscriber/src/fmt/format/pretty.rs @@ -394,11 +394,7 @@ impl<'a> PrettyVisitor<'a> { } pub(crate) fn with_style(self, style: Style) -> Self { - if self.writer.has_ansi_escapes() { - Self { style, ..self } - } else { - self - } + Self { style, ..self } } #[must_use] @@ -411,6 +407,14 @@ impl<'a> PrettyVisitor<'a> { } } + fn style(&self) -> Style { + if self.writer.has_ansi_escapes() { + self.style + } else { + Style::new() + } + } + fn bold_style(&self) -> Style { if self.writer.has_ansi_escapes() { self.style.bold() @@ -461,9 +465,9 @@ impl<'a> field::Visit for PrettyVisitor<'a> { } self.result = if field.name() == "message" { - self.record_debug_impl(field, &format_args!("{}", self.style.paint(value))) + self.record_debug_impl(field, &format_args!("{}", self.style().paint(value))) } else { - self.record_debug_impl(field, &self.style.paint(value)) + self.record_debug_impl(field, &self.style().paint(value)) } } @@ -472,22 +476,23 @@ impl<'a> field::Visit for PrettyVisitor<'a> { return; } + let style = self.style(); self.result = if let Some(source) = value.source() { - let bold: Style = self.bold_style(); + let bold = self.bold_style(); self.record_debug_impl( field, &format_args!( "{}{} {}{}{} {}", - self.style.paint(value), - self.style.paint(","), + style.paint(value), + style.paint(","), bold.paint(field), bold.paint(".sources"), - self.style.paint(":"), - self.style.paint(ErrorSourceList(source)) + style.paint(":"), + style.paint(ErrorSourceList(source)) ), ) } else { - self.record_debug_impl(field, &format_args!("{}", self.style.paint(value))) + self.record_debug_impl(field, &format_args!("{}", style.paint(value))) } } @@ -495,7 +500,7 @@ impl<'a> field::Visit for PrettyVisitor<'a> { if self.result.is_err() { return; } - self.result = self.record_debug_impl(field, &self.style.paint(value)); + self.result = self.record_debug_impl(field, &self.style().paint(value)); } } From b4ecfb767425b9ee3a0ebe7feabaf73ee029ef23 Mon Sep 17 00:00:00 2001 From: Stefnotch Date: Tue, 26 Mar 2024 12:00:24 +0100 Subject: [PATCH 11/12] Simplify printing --- tracing-subscriber/src/filter/env/builder.rs | 27 ++++++++------------ 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/tracing-subscriber/src/filter/env/builder.rs b/tracing-subscriber/src/filter/env/builder.rs index f1fc98deb1..9583dd764e 100644 --- a/tracing-subscriber/src/filter/env/builder.rs +++ b/tracing-subscriber/src/filter/env/builder.rs @@ -217,33 +217,28 @@ impl Builder { // may or may not use a `MakeWriter`. let warn = |msg: &str| { #[cfg(not(feature = "ansi"))] - let msg = format!("warning: {}", msg); + eprintln!("warning: {}", msg); #[cfg(feature = "ansi")] - let msg = { format!("{}{} {}", "warning".yellow().bold(), ":".bold(), msg.bold()) }; - eprintln!("{}", msg); + eprintln!("{}{} {}", "warning".yellow().bold(), ":".bold(), msg.bold()); }; let ctx_prefixed = |prefix: &str, msg: &str| { #[cfg(not(feature = "ansi"))] - let msg = format!("{} {}", prefix, msg); + eprintln!("{} {}", prefix, msg); #[cfg(feature = "ansi")] - let msg = { - format!( - " {} {} {}", - "=".color(XtermColors::Blue).bold(), - prefix.bold(), - msg - ) - }; - eprintln!("{}", msg); + eprintln!( + " {} {} {}", + "=".color(XtermColors::Blue).bold(), + prefix.bold(), + msg + ); }; let ctx_help = |msg| ctx_prefixed("help:", msg); let ctx_note = |msg| ctx_prefixed("note:", msg); let ctx = |msg: &str| { #[cfg(not(feature = "ansi"))] - let msg = format!("note: {}", msg); + eprintln!("note: {}", msg); #[cfg(feature = "ansi")] - let msg = { format!(" {} {}", "|".color(XtermColors::Blue).bold(), msg) }; - eprintln!("{}", msg); + eprintln!(" {} {}", "|".color(XtermColors::Blue).bold(), msg); }; warn("some trace filter directives would enable traces that are disabled statically"); for directive in disabled { From bd7dd8701f951eb264a815c118d537ba01f99ba5 Mon Sep 17 00:00:00 2001 From: Stefnotch Date: Sat, 30 Mar 2024 16:05:09 +0100 Subject: [PATCH 12/12] Fix #2268 --- tracing-subscriber/src/fmt/format/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tracing-subscriber/src/fmt/format/mod.rs b/tracing-subscriber/src/fmt/format/mod.rs index 6fafe89479..a6a7750800 100644 --- a/tracing-subscriber/src/fmt/format/mod.rs +++ b/tracing-subscriber/src/fmt/format/mod.rs @@ -644,7 +644,6 @@ impl Format { /// tracing_subscriber::fmt() /// .pretty() /// .with_ansi(false) - /// .fmt_fields(format::PrettyFields::new().with_ansi(false)) /// // ... other settings ... /// .init(); /// ```