From aa38daa27483d4d406f11424304dfcac74bbac96 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Mon, 8 Nov 2021 16:21:43 -0800 Subject: [PATCH] subscriber: use ANSI formatting in field formatters (#1702) Depends on #1696 ## Motivation PR #1696 adds a new API for propagating whether or not ANSI formatting escape codes are enabled to the event formatter and field formatter via the `Writer` type. This means that it's now quite easy to make field formatting also include ANSI formatting escape codes when ANSI formatting is enabled. Currently, `tracing-subscriber`'s default field formatter does not use ANSI formatting --- ANSI escape codes are currently only used for parts of the event log line controlled by the event formatter, not within fields. Adding ANSI colors and formatting in the fields could make the formatted output nicer looking and easier to read. ## Solution This branch adds support for ANSI formatting escapes in the default field formatter, when ANSI formatting is enabled. Now, field names will be italicized, and the `=` delimiter is dimmed. This should make it a little easier to visually scan the field lists, since the names and values are more clearly separated and should be easier to distinguish. Additionally, I cleaned up the code for conditionally using ANSI formatting significantly. Now, the `Writer` type has (crate-private) methods for returning `Style`s for bold, dimmed, and italicized text, when ANSI escapes are enabled, or no-op `Style`s when they are not. This is reused in all the formatter implementations, so it makes sense to have it in one place. I also added dimmed formatting of delimiters in a couple other places in the default event format, which I thought improved readability a bit by bringing the actual *text* to the forefront. Example of the default format with ANSI escapes enabled: ![image](https://user-images.githubusercontent.com/2796466/140166750-aaf64bd8-b051-4985-9e7d-168fe8757b11.png) Signed-off-by: Eliza Weisman --- tracing-subscriber/src/fmt/format/mod.rs | 119 ++++++++++++++------ tracing-subscriber/src/fmt/format/pretty.rs | 6 +- 2 files changed, 84 insertions(+), 41 deletions(-) diff --git a/tracing-subscriber/src/fmt/format/mod.rs b/tracing-subscriber/src/fmt/format/mod.rs index e2332f2af5..ae2bdb6d59 100644 --- a/tracing-subscriber/src/fmt/format/mod.rs +++ b/tracing-subscriber/src/fmt/format/mod.rs @@ -376,6 +376,39 @@ impl<'writer> Writer<'writer> { pub fn has_ansi_escapes(&self) -> bool { self.is_ansi } + + pub(in crate::fmt::format) fn bold(&self) -> Style { + #[cfg(feature = "ansi")] + { + if self.is_ansi { + return Style::new().bold(); + } + } + + Style::new() + } + + pub(in crate::fmt::format) fn dimmed(&self) -> Style { + #[cfg(feature = "ansi")] + { + if self.is_ansi { + return Style::new().dimmed(); + } + } + + Style::new() + } + + pub(in crate::fmt::format) fn italic(&self) -> Style { + #[cfg(feature = "ansi")] + { + if self.is_ansi { + return Style::new().italic(); + } + } + + Style::new() + } } impl fmt::Write for Writer<'_> { @@ -724,12 +757,11 @@ where write!(writer, "{:0>2?} ", std::thread::current().id())?; } + let dimmed = writer.dimmed(); + if let Some(scope) = ctx.ctx.event_scope(event) { - let bold = if writer.has_ansi_escapes() { - Style::new().bold() - } else { - Style::new() - }; + let bold = writer.bold(); + let mut seen = false; for span in scope.from_root() { @@ -742,7 +774,7 @@ where write!(writer, "{}{}{}", bold.paint("{"), fields, bold.paint("}"))?; } } - writer.write_char(':')?; + write!(writer, "{}", dimmed.paint(":"))?; } if seen { @@ -751,7 +783,12 @@ where }; if self.display_target { - write!(writer, "{}: ", meta.target())?; + write!( + writer, + "{}{} ", + dimmed.paint(meta.target()), + dimmed.paint(":") + )?; } ctx.format_fields(writer.by_ref(), event)?; @@ -821,15 +858,12 @@ where } if self.display_target { - let target = meta.target(); - #[cfg(feature = "ansi")] - let target = if writer.has_ansi_escapes() { - Style::new().bold().paint(target) - } else { - Style::new().paint(target) - }; - - write!(writer, "{}:", target)?; + write!( + writer, + "{}{}", + writer.bold().paint(meta.target()), + writer.dimmed().paint(":") + )?; } let fmt_ctx = { @@ -844,25 +878,18 @@ where }; write!(writer, "{}", fmt_ctx)?; - let span = event - .parent() - .and_then(|id| ctx.ctx.span(id)) - .or_else(|| ctx.ctx.lookup_current()); - - let scope = span.into_iter().flat_map(|span| span.scope()); - #[cfg(feature = "ansi")] - let dimmed = if writer.has_ansi_escapes() { - Style::new().dimmed() - } else { - Style::new() - }; - for span in scope { + let dimmed = writer.dimmed(); + for span in ctx + .ctx + .event_scope(event) + .into_iter() + .map(crate::registry::Scope::from_root) + .flatten() + { let exts = span.extensions(); if let Some(fields) = exts.get::>() { if !fields.is_empty() { - #[cfg(feature = "ansi")] - let fields = dimmed.paint(fields.as_str()); - write!(writer, " {}", fields)?; + write!(writer, " {}", dimmed.paint(&fields.fields))?; } } } @@ -969,9 +996,17 @@ 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(); self.record_debug( field, - &format_args!("{} {}.sources={}", value, field, ErrorSourceList(source)), + &format_args!( + "{} {}{}{}{}", + value, + italic.paint(field.name()), + italic.paint(".sources"), + self.writer.dimmed().paint("="), + ErrorSourceList(source) + ), ) } else { self.record_debug(field, &format_args!("{}", value)) @@ -989,8 +1024,20 @@ impl<'a> field::Visit for DefaultVisitor<'a> { // 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#") => write!(self.writer, "{}={:?}", &name[2..], value), - name => write!(self.writer, "{}={:?}", name, value), + name if name.starts_with("r#") => write!( + self.writer, + "{}{}{:?}", + self.writer.italic().paint(&name[2..]), + self.writer.dimmed().paint("="), + value + ), + name => write!( + self.writer, + "{}{}{:?}", + self.writer.italic().paint(name), + self.writer.dimmed().paint("="), + value + ), }; } } @@ -1459,7 +1506,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 tracing_subscriber::fmt::format::test: hello\n"; + 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"; test_ansi(true, expected); } diff --git a/tracing-subscriber/src/fmt/format/pretty.rs b/tracing-subscriber/src/fmt/format/pretty.rs index cea20c86fd..80e6d2366a 100644 --- a/tracing-subscriber/src/fmt/format/pretty.rs +++ b/tracing-subscriber/src/fmt/format/pretty.rs @@ -187,11 +187,7 @@ where writer.write_char('\n')?; } - let bold = if writer.has_ansi_escapes() { - Style::new().bold() - } else { - Style::new() - }; + let bold = writer.bold(); let span = event .parent() .and_then(|id| ctx.span(id))