From 8a7d0c521c81fd56c6405bac2e5b771f42d8c2a9 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Mon, 8 Nov 2021 10:40:19 -0800 Subject: [PATCH] subscriber: propagate ANSI config via `Writer` (#1696) ## Motivation Currently, whether `tracing-subscriber`'s `fmt` subscriber will ANSI formatting escape codes use is configured on the `Format` type. This means that the configuration is honored by the event formatters implemented in `tracing-subscriber`, but is not easily exposed to those in other crates. Additionally, it's not currently easy to expose the configuration to the field formatter, so it's difficult to implement field formatters that use ANSI escape codes conditionally. Issue #1651 suggested a new API for this, where the writer that's passed in to the event and field formatters provides a method for checking if ANSI escape codes are supported. ## Solution This branch adds a new method to the `Writer` type added in #1661. The `FormatEvent` and `FormatFields` implementations can call `Writer::has_ansi_escapes` to determine whether ANSI escape codes are supported. This is also propagated to `FormattedFields`, so that it can be determined when adding new fields to a preexisting set of formatted fields. Fixes #1661 Signed-off-by: Eliza Weisman --- tracing-subscriber/src/fmt/fmt_subscriber.rs | 49 ++++++++++----- tracing-subscriber/src/fmt/format/mod.rs | 57 ++++++++++++++---- tracing-subscriber/src/fmt/format/pretty.rs | 63 +++++++------------- 3 files changed, 101 insertions(+), 68 deletions(-) diff --git a/tracing-subscriber/src/fmt/fmt_subscriber.rs b/tracing-subscriber/src/fmt/fmt_subscriber.rs index 48fe9a8718..304417f949 100644 --- a/tracing-subscriber/src/fmt/fmt_subscriber.rs +++ b/tracing-subscriber/src/fmt/fmt_subscriber.rs @@ -67,6 +67,7 @@ pub struct Subscriber, } @@ -114,6 +115,7 @@ where fmt_event: e, fmt_span: self.fmt_span, make_writer: self.make_writer, + is_ansi: self.is_ansi, _inner: self._inner, } } @@ -148,6 +150,7 @@ impl Subscriber { fmt_fields: self.fmt_fields, fmt_event: self.fmt_event, fmt_span: self.fmt_span, + is_ansi: self.is_ansi, make_writer, _inner: self._inner, } @@ -180,10 +183,21 @@ impl Subscriber { fmt_fields: self.fmt_fields, fmt_event: self.fmt_event, fmt_span: self.fmt_span, + is_ansi: self.is_ansi, make_writer: TestWriter::default(), _inner: self._inner, } } + + /// Enable ANSI terminal colors for formatted output. + #[cfg(feature = "ansi")] + #[cfg_attr(docsrs, doc(cfg(feature = "ansi")))] + pub fn with_ansi(self, ansi: bool) -> Self { + Subscriber { + is_ansi: ansi, + ..self + } + } } impl Subscriber, W> @@ -210,6 +224,7 @@ where fmt_fields: self.fmt_fields, fmt_span: self.fmt_span, make_writer: self.make_writer, + is_ansi: self.is_ansi, _inner: self._inner, } } @@ -221,6 +236,7 @@ where fmt_fields: self.fmt_fields, fmt_span: self.fmt_span.without_time(), make_writer: self.make_writer, + is_ansi: self.is_ansi, _inner: self._inner, } } @@ -273,16 +289,6 @@ where } } - /// Enable ANSI terminal colors for formatted output. - #[cfg(feature = "ansi")] - #[cfg_attr(docsrs, doc(cfg(feature = "ansi")))] - pub fn with_ansi(self, ansi: bool) -> Subscriber, W> { - Subscriber { - fmt_event: self.fmt_event.with_ansi(ansi), - ..self - } - } - /// Sets whether or not an event's target is displayed. pub fn with_target(self, display_target: bool) -> Subscriber, W> { Subscriber { @@ -337,6 +343,7 @@ where fmt_fields: self.fmt_fields, fmt_span: self.fmt_span, make_writer: self.make_writer, + is_ansi: self.is_ansi, _inner: self._inner, } } @@ -350,6 +357,7 @@ where fmt_fields: format::Pretty::default(), fmt_span: self.fmt_span, make_writer: self.make_writer, + is_ansi: self.is_ansi, _inner: self._inner, } } @@ -377,6 +385,8 @@ where fmt_fields: format::JsonFields::new(), fmt_span: self.fmt_span, make_writer: self.make_writer, + // always disable ANSI escapes in JSON mode! + is_ansi: false, _inner: self._inner, } } @@ -442,6 +452,7 @@ impl Subscriber { fmt_fields, fmt_span: self.fmt_span, make_writer: self.make_writer, + is_ansi: self.is_ansi, _inner: self._inner, } } @@ -454,6 +465,7 @@ impl Default for Subscriber { fmt_event: format::Format::default(), fmt_span: format::FmtSpanConfig::default(), make_writer: io::stdout, + is_ansi: cfg!(feature = "ansi"), _inner: PhantomData, } } @@ -487,6 +499,7 @@ where #[derive(Default)] pub struct FormattedFields { _format_fields: PhantomData, + was_ansi: bool, /// The formatted fields of a span. pub fields: String, } @@ -496,6 +509,7 @@ impl FormattedFields { pub fn new(fields: String) -> Self { Self { fields, + was_ansi: false, _format_fields: PhantomData, } } @@ -505,7 +519,7 @@ impl FormattedFields { /// The returned [`format::Writer`] can be used with the /// [`FormatFields::format_fields`] method. pub fn as_writer(&mut self) -> format::Writer<'_> { - format::Writer::new(&mut self.fields) + format::Writer::new(&mut self.fields).with_ansi(self.was_ansi) } } @@ -514,6 +528,7 @@ impl fmt::Debug for FormattedFields { f.debug_struct("FormattedFields") .field("fields", &self.fields) .field("formatter", &format_args!("{}", std::any::type_name::())) + .field("was_ansi", &self.was_ansi) .finish() } } @@ -564,9 +579,10 @@ where let mut fields = FormattedFields::::new(String::new()); if self .fmt_fields - .format_fields(fields.as_writer(), attrs) + .format_fields(fields.as_writer().with_ansi(self.is_ansi), attrs) .is_ok() { + fields.was_ansi = self.is_ansi; extensions.insert(fields); } } @@ -598,9 +614,10 @@ where let mut fields = FormattedFields::::new(String::new()); if self .fmt_fields - .format_fields(fields.as_writer(), values) + .format_fields(fields.as_writer().with_ansi(self.is_ansi), values) .is_ok() { + fields.was_ansi = self.is_ansi; extensions.insert(fields); } } @@ -705,7 +722,11 @@ where let ctx = self.make_ctx(ctx); if self .fmt_event - .format_event(&ctx, format::Writer::new(&mut buf), event) + .format_event( + &ctx, + format::Writer::new(&mut buf).with_ansi(self.is_ansi), + event, + ) .is_ok() { let mut writer = self.make_writer.make_writer_for(event.metadata()); diff --git a/tracing-subscriber/src/fmt/format/mod.rs b/tracing-subscriber/src/fmt/format/mod.rs index ef32b30ba7..bc7eb40f2c 100644 --- a/tracing-subscriber/src/fmt/format/mod.rs +++ b/tracing-subscriber/src/fmt/format/mod.rs @@ -234,6 +234,7 @@ where pub struct Writer<'writer> { writer: &'writer mut dyn fmt::Write, // TODO(eliza): add ANSI support + is_ansi: bool, } /// A [`FormatFields`] implementation that formats fields by calling a function @@ -273,7 +274,7 @@ pub struct Full; pub struct Format { format: F, pub(crate) timer: T, - pub(crate) ansi: bool, + pub(crate) ansi: Option, pub(crate) display_timestamp: bool, pub(crate) display_target: bool, pub(crate) display_level: bool, @@ -291,16 +292,26 @@ impl<'writer> Writer<'writer> { pub(crate) fn new(writer: &'writer mut impl fmt::Write) -> Self { Self { writer: writer as &mut dyn fmt::Write, + is_ansi: false, } } + // TODO(eliza): consider making this a public API? + pub(crate) fn with_ansi(self, is_ansi: bool) -> Self { + Self { is_ansi, ..self } + } + /// Return a new `Writer` that mutably borrows `self`. /// /// This can be used to temporarily borrow a `Writer` to pass a new `Writer` /// to a function that takes a `Writer` by value, allowing the original writer /// to still be used once that function returns. pub fn by_ref(&mut self) -> Writer<'_> { - Writer::new(self) + let is_ansi = self.is_ansi; + Writer { + writer: self as &mut dyn fmt::Write, + is_ansi, + } } /// Writes a string slice into this `Writer`, returning whether the write succeeded. @@ -346,7 +357,7 @@ impl<'writer> Writer<'writer> { self.writer.write_char(c) } - /// Glue for usage of the [`write!`] macro with `Wrriter`s. + /// Glue for usage of the [`write!`] macro with `Writer`s. /// /// This method should generally not be invoked manually, but rather through /// the [`write!`] macro itself. @@ -361,6 +372,14 @@ impl<'writer> Writer<'writer> { pub fn write_fmt(&mut self, args: fmt::Arguments<'_>) -> fmt::Result { self.writer.write_fmt(args) } + + /// Returns `true` if ANSI escape codes may be used to add colors + /// and other formatting when writing to this `Writer`. + /// + /// If this returns `false`, formatters should not emit ANSI escape codes. + pub fn has_ansi_escapes(&self) -> bool { + self.is_ansi + } } impl fmt::Write for Writer<'_> { @@ -384,6 +403,7 @@ impl fmt::Debug for Writer<'_> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("Writer") .field("writer", &format_args!("<&mut dyn fmt::Write>")) + .field("is_ansi", &self.is_ansi) .finish() } } @@ -395,7 +415,7 @@ impl Default for Format { Format { format: Full, timer: SystemTime, - ansi: true, + ansi: None, display_timestamp: true, display_target: true, display_level: true, @@ -532,7 +552,10 @@ impl Format { /// Enable ANSI terminal colors for formatted output. pub fn with_ansi(self, ansi: bool) -> Format { - Format { ansi, ..self } + Format { + ansi: Some(ansi), + ..self + } } /// Sets whether or not an event's target is displayed. @@ -573,7 +596,7 @@ impl Format { } } - fn format_level(&self, level: Level, writer: &mut dyn fmt::Write) -> fmt::Result + fn format_level(&self, level: Level, writer: &mut Writer<'_>) -> fmt::Result where F: LevelNames, { @@ -581,7 +604,7 @@ impl Format { let fmt_level = { #[cfg(feature = "ansi")] { - F::format_level(level, self.ansi) + F::format_level(level, writer.has_ansi_escapes()) } #[cfg(not(feature = "ansi"))] { @@ -608,7 +631,7 @@ impl Format { // colors. #[cfg(feature = "ansi")] { - if self.ansi { + if writer.has_ansi_escapes() { let style = Style::new().dimmed(); write!(writer, "{}", style.prefix())?; @@ -632,10 +655,10 @@ impl Format { writer.write_char(' ') } - fn bold(&self) -> Style { + fn bold(&self, is_ansi: bool) -> Style { #[cfg(feature = "ansi")] { - if self.ansi { + if self.ansi.unwrap_or(true) && is_ansi { return Style::new().bold(); } } @@ -704,6 +727,14 @@ where #[cfg(not(feature = "tracing-log"))] let meta = event.metadata(); + // if the `Format` struct *also* has an ANSI color configuration, + // override the writer...the API for configuring ANSI color codes on the + // `Format` struct is deprecated, but we still need to honor those + // configurations. + if let Some(ansi) = self.ansi { + writer = writer.with_ansi(ansi); + } + self.format_timestamp(&mut writer)?; self.format_level(*meta.level(), &mut writer)?; @@ -726,7 +757,7 @@ where } if let Some(scope) = ctx.ctx.event_scope(event) { - let bold = self.bold(); + let bold = self.bold(writer.has_ansi_escapes()); let mut seen = false; for span in scope.from_root() { @@ -799,7 +830,7 @@ where if self.display_target { let target = meta.target(); #[cfg(feature = "ansi")] - let target = if self.ansi { + let target = if writer.has_ansi_escapes() { Style::new().bold().paint(target) } else { Style::new().paint(target) @@ -811,7 +842,7 @@ where ctx.format_fields(writer.by_ref(), event)?; #[cfg(feature = "ansi")] - let dimmed = if self.ansi { + let dimmed = if writer.has_ansi_escapes() { Style::new().dimmed() } else { Style::new() diff --git a/tracing-subscriber/src/fmt/format/pretty.rs b/tracing-subscriber/src/fmt/format/pretty.rs index 78acbcc04e..8629b05183 100644 --- a/tracing-subscriber/src/fmt/format/pretty.rs +++ b/tracing-subscriber/src/fmt/format/pretty.rs @@ -5,7 +5,7 @@ use crate::{ registry::LookupSpan, }; -use std::fmt::{self, Write}; +use std::fmt; use tracing_core::{ field::{self, Field}, Collect, Event, Level, @@ -26,10 +26,10 @@ pub struct Pretty { /// /// [visitor]: field::Visit /// [`MakeVisitor`]: crate::field::MakeVisitor +#[derive(Debug)] pub struct PrettyVisitor<'a> { - writer: &'a mut dyn Write, + writer: Writer<'a>, is_empty: bool, - ansi: bool, style: Style, result: fmt::Result, } @@ -107,7 +107,7 @@ where self.format_timestamp(&mut writer)?; - let style = if self.display_level && self.ansi { + let style = if self.display_level && writer.has_ansi_escapes() { Pretty::style_for(meta.level()) } else { Style::new() @@ -118,7 +118,11 @@ where } if self.display_target { - let target_style = if self.ansi { style.bold() } else { style }; + let target_style = if writer.has_ansi_escapes() { + style.bold() + } else { + style + }; write!( writer, "{}{}{}: ", @@ -127,14 +131,12 @@ where target_style.infix(style) )?; } - let mut v = PrettyVisitor::new(&mut writer, true) - .with_style(style) - .with_ansi(self.ansi); + let mut v = PrettyVisitor::new(writer.by_ref(), true).with_style(style); event.record(&mut v); v.finish()?; writer.write_char('\n')?; - let dimmed = if self.ansi { + let dimmed = if writer.has_ansi_escapes() { Style::new().dimmed().italic() } else { Style::new() @@ -173,7 +175,7 @@ where writer.write_char('\n')?; } - let bold = if self.ansi { + let bold = if writer.has_ansi_escapes() { Style::new().bold() } else { Style::new() @@ -219,12 +221,8 @@ where } impl<'writer> FormatFields<'writer> for Pretty { - fn format_fields( - &self, - mut writer: Writer<'writer>, - fields: R, - ) -> fmt::Result { - let mut v = PrettyVisitor::new(&mut writer, false); + fn format_fields(&self, writer: Writer<'writer>, fields: R) -> fmt::Result { + let mut v = PrettyVisitor::new(writer, false); fields.record(&mut v); v.finish() } @@ -235,8 +233,8 @@ impl<'writer> FormatFields<'writer> for Pretty { fields: &span::Record<'_>, ) -> fmt::Result { let empty = current.is_empty(); - let mut writer = current.as_writer(); - let mut v = PrettyVisitor::new(&mut writer, empty); + let writer = current.as_writer(); + let mut v = PrettyVisitor::new(writer, empty); fields.record(&mut v); v.finish() } @@ -267,7 +265,7 @@ impl<'a> MakeVisitor> for PrettyFields { #[inline] fn make_visitor(&self, target: Writer<'a>) -> Self::Visitor { - PrettyVisitor::new(target.writer, true).with_ansi(self.ansi) + PrettyVisitor::new(target.with_ansi(self.ansi), true) } } @@ -280,11 +278,10 @@ impl<'a> PrettyVisitor<'a> { /// - `writer`: the writer to format to. /// - `is_empty`: whether or not any fields have been previously written to /// that writer. - pub fn new(writer: &'a mut dyn Write, is_empty: bool) -> Self { + pub fn new(writer: Writer<'a>, is_empty: bool) -> Self { Self { writer, is_empty, - ansi: true, style: Style::default(), result: Ok(()), } @@ -294,10 +291,6 @@ impl<'a> PrettyVisitor<'a> { Self { style, ..self } } - pub(crate) fn with_ansi(self, ansi: bool) -> Self { - Self { ansi, ..self } - } - fn write_padded(&mut self, value: &impl fmt::Debug) { let padding = if self.is_empty { self.is_empty = false; @@ -309,7 +302,7 @@ impl<'a> PrettyVisitor<'a> { } fn bold(&self) -> Style { - if self.ansi { + if self.writer.has_ansi_escapes() { self.style.bold() } else { Style::new() @@ -378,26 +371,14 @@ impl<'a> field::Visit for PrettyVisitor<'a> { } impl<'a> VisitOutput for PrettyVisitor<'a> { - fn finish(self) -> fmt::Result { - write!(self.writer, "{}", self.style.suffix())?; + fn finish(mut self) -> fmt::Result { + write!(&mut self.writer, "{}", self.style.suffix())?; self.result } } impl<'a> VisitFmt for PrettyVisitor<'a> { fn writer(&mut self) -> &mut dyn fmt::Write { - self.writer - } -} - -impl<'a> fmt::Debug for PrettyVisitor<'a> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("PrettyVisitor") - .field("writer", &format_args!("")) - .field("is_empty", &self.is_empty) - .field("result", &self.result) - .field("style", &self.style) - .field("ansi", &self.ansi) - .finish() + &mut self.writer } }