Fix NO_COLOR to only suppress colors, not all ANSI styles

The NO_COLOR convention (https://no-color.org) only calls for
suppressing ANSI *color* codes. It does not demand the removal of
other attributes such as bold, italic, underline or reverse.

Previously, Paint::is_enabled() bundled anstyle_query::no_color()
into the single boolean that decides whether to emit any formatting.
Because Paint relies on that boolean alone, NO_COLOR ended up
stripping bold, italic, reverse, etc. alongside the color codes.
That conflates two concerns: terminal styling support and color
suppression. It is also contrary to the intent of the standard.

Instead, treat the general style decision separately from the
color decision.  Remove the no_color() check from is_enabled()
and add Paint::is_styling_enabled() for the coarse terminal gate.
Style::fmt_prefix and fmt_suffix now query NO_COLOR directly and
conditionally drop foreground and background codes while preserving
non-colour properties.  This way bold, italic, underline and the
like continue to work when NO_COLOR is set, while colors alone are
suppressed.

Closes: rad:z3gqcJUoA1n9HaHKufZs5FCSGazv5/cob/xyz.radicle.issue/f922d90
Assisted-by: Pi:deepinfra/moonshotai/Kimi-K2.6
Signed-off-by: Richard Levitte <richard@levitte.org>

crates/radicle-term/src/ansi/paint.rs

@@ -261,7 +261,11 @@ impl<T> Paint<T> {
 
 impl<T: fmt::Display> fmt::Display for Paint<T> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        if Paint::is_enabled() && self.style.wrap {
+        if !Paint::is_enabled() {
+            return fmt::Display::fmt(&self.item, f);
+        }
+
+        if self.style.wrap {
             let mut prefix = String::new();
             prefix.push_str("\x1B[0m");
             self.style.fmt_prefix(&mut prefix)?;
@@ -270,19 +274,20 @@ impl<T: fmt::Display> fmt::Display for Paint<T> {
             let item = format!("{}", self.item).replace("\x1B[0m", &prefix);
             fmt::Display::fmt(&item, f)?;
             self.style.fmt_suffix(f)
-        } else if Paint::is_enabled() {
+        } else {
             self.style.fmt_prefix(f)?;
             fmt::Display::fmt(&self.item, f)?;
             self.style.fmt_suffix(f)
-        } else {
-            fmt::Display::fmt(&self.item, f)
         }
     }
 }
 
 impl Paint<()> {
-    /// Returns `true` if coloring is enabled and `false` otherwise.
+    /// Returns `true` if ANSI styling (bold, italic, underline, reverse, etc.)
-    pub fn is_enabled() -> bool {
+    /// is enabled. This reflects the general terminal capability and user
+    /// preferences governed by `CLICOLOR`, `ENABLED` / `FORCED`, and terminal
+    /// detection. It does **not** take `NO_COLOR` into account.
+    pub fn is_styling_enabled() -> bool {
         if FORCED.load(sync::atomic::Ordering::SeqCst) {
             return true;
         }
@@ -295,12 +300,18 @@ impl Paint<()> {
 
         is_terminal
             && is_enabled
-            && !anstyle_query::no_color()
             && !clicolor_disabled
             && (anstyle_query::term_supports_color() || clicolor_enabled || anstyle_query::is_ci())
             || anstyle_query::clicolor_force()
     }
 
+    /// Returns `true` if any painting is enabled.
+    ///
+    /// This is equivalent to `is_styling_enabled()`.
+    pub fn is_enabled() -> bool {
+        Self::is_styling_enabled()
+    }
+
     /// Check 24-bit RGB color support.
     pub fn truecolor() -> bool {
         static TRUECOLOR: LazyLock<bool> = LazyLock::new(anstyle_query::term_supports_color);

crates/radicle-term/src/ansi/style.rs

@@ -21,6 +21,11 @@ impl Property {
         Property(0)
     }
 
+    #[inline(always)]
+    pub const fn is_empty(self) -> bool {
+        self.0 == 0
+    }
+
     #[inline(always)]
     pub const fn contains(self, other: Property) -> bool {
         (other.0 & self.0) == other.0
@@ -235,10 +240,16 @@ impl Style {
     /// is currently enabled or disabled. To write the prefix only if painting
     /// is enabled, condition a call to this method on [`Paint::is_enabled()`].
     pub fn fmt_prefix(&self, f: &mut dyn fmt::Write) -> fmt::Result {
+        let color_enabled = !anstyle_query::no_color();
+
         // A user may just want a code-free string when no styles are applied.
         if self.is_plain() {
             return Ok(());
         }
+        // When colors are disabled and there are no non-color properties, there's nothing to emit.
+        if !color_enabled && self.properties.is_empty() {
+            return Ok(());
+        }
 
         let mut splice = false;
         write!(f, "\x1B[")?;
@@ -248,6 +259,7 @@ impl Style {
             write_spliced(&mut splice, f, k)?;
         }
 
+        if color_enabled {
             if self.background != Color::Unset {
                 write_spliced(&mut splice, f, "4")?;
                 self.background.ansi_fmt(f)?;
@@ -257,6 +269,7 @@ impl Style {
                 write_spliced(&mut splice, f, "3")?;
                 self.foreground.ansi_fmt(f)?;
             }
+        }
 
         // All the codes end with an `m`.
         write!(f, "m")
@@ -272,9 +285,16 @@ impl Style {
     /// is currently enabled or disabled. To write the suffix only if painting
     /// is enabled, condition a call to this method on [`Paint::is_enabled()`].
     pub fn fmt_suffix(&self, f: &mut dyn fmt::Write) -> fmt::Result {
+        let color_enabled = !anstyle_query::no_color();
+
+        // A user may just want a code-free string when no styles are applied.
         if self.is_plain() {
             return Ok(());
         }
+        // When colors are disabled and there are no non-color properties, there's nothing to emit.
+        if !color_enabled && self.properties.is_empty() {
+            return Ok(());
+        }
         write!(f, "\x1B[0m")
     }
 }

crates/radicle-term/src/ansi/tests.rs

@@ -278,3 +278,65 @@ fn wrapping() {
             .to_string()
     );
 }
+
+#[test]
+fn no_color_disables_colors_not_styles() {
+    let _guard = SERIAL.lock();
+
+    Paint::force(true);
+    Paint::enable();
+
+    // Without NO_COLOR, colors and styles are both emitted.
+    assert_eq!(
+        Paint::red("hi").bold().to_string(),
+        "\x1B[1;31mhi\x1B[0m".to_string()
+    );
+
+    unsafe { std::env::set_var("NO_COLOR", "1") }
+
+    // With NO_COLOR, foreground and background colors are suppressed,
+    // but bold, italic, underline, invert etc. are preserved.
+    assert_eq!(
+        Paint::red("hi").bold().to_string(),
+        "\x1B[1mhi\x1B[0m".to_string()
+    );
+    assert_eq!(Paint::red("hi").to_string(), "hi".to_string());
+    assert_eq!(
+        Paint::new("hi").bold().to_string(),
+        "\x1B[1mhi\x1B[0m".to_string()
+    );
+    assert_eq!(
+        Paint::new("hi").italic().to_string(),
+        "\x1B[3mhi\x1B[0m".to_string()
+    );
+    assert_eq!(
+        Paint::new("hi").invert().to_string(),
+        "\x1B[7mhi\x1B[0m".to_string()
+    );
+
+    // Wrapping without colors should still preserve inner style resets.
+    let inner = || {
+        format!(
+            "{} b {}",
+            Paint::red("a").bold(),
+            Paint::green("c").underline()
+        )
+    };
+    assert_eq!(
+        Paint::new(&inner()).wrap().to_string(),
+        "\u{1b}[1ma\u{1b}[0m b \u{1b}[4mc\u{1b}[0m".to_string()
+    );
+
+    unsafe { std::env::remove_var("NO_COLOR") }
+
+    // When all styling is explicitly disabled, no ANSI codes are emitted at all.
+    Paint::force(false);
+    Paint::disable();
+    assert_eq!(Paint::red("hi").bold().to_string(), "hi".to_string());
+    assert_eq!(Paint::new("hi").bold().to_string(), "hi".to_string());
+    assert_eq!(Paint::new("hi").italic().to_string(), "hi".to_string());
+    assert_eq!(Paint::new("hi").invert().to_string(), "hi".to_string());
+    assert_eq!(Paint::blue("hi").underline().to_string(), "hi".to_string());
+    Paint::force(true);
+    Paint::enable();
+}