subscriber: make ANSI and not-ANSI formatting consistent

Currently, the `tracing-subscriber` crate's `fmt` module has two
separate `fmt::Display` impls for its' `FmtCtx` and `FullCtx` types,
depending on whether the feature flag that enables ANSI colors is set.
Since a lot of the `fmt::Display` implementations doesn't have anything
to do with ANSI colors, this means that we have a lot of repeated code,
and in order to keep the formatting consistent, we have to keep these
implementations in sync manually. There are currently some
inconsistencies in formatting with ANSI colors on and off, implying that
we have failed to do that.

This commit simplifies the situation significantly by consolidating the
`fmt::Display` impls into one implementation, and only feature flagging
the code that actually needs to be different. We do this by introducing
a `Style` type which exposes no-op versions of methods with the same
names as the `ansi-term` crate's `Style` when ANSI formatting is
disabled. Now, the conditional logic is hidden in the function that
returns a `Style`, and the rest of the `fmt::Display` implementations
can be agnostic of ANSI colors. In release mode, `rustc` should be able
to optimize out the no-op methods and empty struct entirely.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

tracing-subscriber/src/fmt/format/mod.rs

@@ -488,53 +488,33 @@ where
     pub(crate) fn new(ctx: &'a FmtContext<'_, S, N>) -> Self {
         Self { ctx }
     }
+
+    fn bold(&self) -> Style {
+        #[cfg(feature = "ansi")]
+        {
+            if self.ansi {
+                return Style::new().bold();
+            }
+        }
+
+        Style::new()
+    }
 }
 
-#[cfg(feature = "ansi")]
 impl<'a, S, N: 'a> fmt::Display for FmtCtx<'a, S, N>
 where
     S: Subscriber + for<'lookup> LookupSpan<'lookup>,
     N: for<'writer> FormatFields<'writer> + 'static,
 {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let bold = self.bold();
         let mut seen = false;
+
         self.ctx.visit_spans(|span| {
-            if seen {
-                f.pad(":")?;
-            }
             seen = true;
-
+            write!(f, "{}:", bold.paint(span.metadata().name()))
-            let metadata = span.metadata();
-            if self.ansi {
-                write!(f, "{}", Style::new().bold().paint(metadata.name()))?;
-            } else {
-                write!(f, "{}", metadata.name())?;
-            }
-
-            Ok(()) as fmt::Result
         })?;
-        if seen {
-            f.pad(" ")?;
-        }
-        Ok(())
-    }
-}
 
-#[cfg(not(feature = "ansi"))]
-impl<'a, S, N> fmt::Display for FmtCtx<'a, S, N>
-where
-    S: Subscriber + for<'lookup> LookupSpan<'lookup>,
-    N: for<'writer> FormatFields<'writer> + 'static,
-{
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let mut seen = false;
-        self.ctx.visit_spans(|span| {
-            if seen {
-                f.pad(":")?;
-            }
-            seen = true;
-            write!(f, "{}", span.name())
-        })?;
         if seen {
             f.pad(" ")?;
         }
@@ -566,33 +546,39 @@ where
     pub(crate) fn new(ctx: &'a FmtContext<'a, S, N>) -> Self {
         Self { ctx }
     }
+
+    fn bold(&self) -> Style {
+        #[cfg(feature = "ansi")] {
+            if self.ansi {
+                return Style::new().bold();
+            }
+        }
+
+
+        Style::new()
+    }
 }
 
-#[cfg(feature = "ansi")]
 impl<'a, S, N> fmt::Display for FullCtx<'a, S, N>
 where
     S: Subscriber + for<'lookup> LookupSpan<'lookup>,
     N: for<'writer> FormatFields<'writer> + 'static,
 {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let bold = self.bold();
         let mut seen = false;
-        let style = if self.ansi {
+
-            Style::new().bold()
-        } else {
-            Style::new()
-        };
         self.ctx.visit_spans(|span| {
-            let metadata = span.metadata();
+            write!(f, "{}", bold.paint(span.metadata().name()))?;
-            write!(f, "{}", style.paint(metadata.name()))?;
             seen = true;
 
             let ext = span.extensions();
-            let data = &ext
+            let fields = &ext
                 .get::<FormattedFields<N>>()
                 .expect("Unable to find FormattedFields in extensions; this is a bug");
-            write!(f, "{}{}{}", style.paint("{"), data, style.paint("}"))?;
+            write!(f, "{}{}{}:", bold.paint("{"), fields, bold.paint("}"))
-            ":".fmt(f)
         })?;
+
         if seen {
             f.pad(" ")?;
         }
@@ -601,27 +587,13 @@ where
 }
 
 #[cfg(not(feature = "ansi"))]
-impl<'a, S, N> fmt::Display for FullCtx<'a, S, N>
+struct Style;
-where
-    S: Subscriber + for<'lookup> LookupSpan<'lookup>,
-    N: for<'writer> FormatFields<'writer> + 'static,
-{
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let mut seen = false;
-        self.ctx.visit_spans(|span| {
-            write!(f, "{}", span.name())?;
-            seen = true;
 
-            let fields = span.fields();
+#[cfg(not(feature = "ansi"))]
-            if !fields.is_empty() {
+impl Style {
-                write!(f, "{{{}}}", fields)?;
+    fn new() -> Self { Style }
-            }
+    fn paint(&self, d: impl fmt::Display) -> impl fmt::Display {
-            ":".fmt(f)
+        d
-        })?;
-        if seen {
-            f.pad(" ")?;
-        }
-        Ok(())
     }
 }