From 07ed77e9a895e8d6bc6afaef487f48bc1323ad00 Mon Sep 17 00:00:00 2001
From: Johannes Altmanninger <aclopte@gmail.com>
Date: Wed, 23 Apr 2025 19:30:22 +0200
Subject: [PATCH] builtin set_color: do print other colors if one is reset-all

Commit cebc05f6c1a (Reset is not a color, 2025-04-15) tried to simpilfy
this behavior but forgot about cases like "set_color --background=normal
red --bold" where we do want to continue after resetting background.

Closes #11417
---
 src/builtins/set_color.rs   | 44 +++++++++++++++++++------------------
 tests/checks/set_color.fish |  2 +-
 2 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/src/builtins/set_color.rs b/src/builtins/set_color.rs
index 82824623e..f2d236803 100644
--- a/src/builtins/set_color.rs
+++ b/src/builtins/set_color.rs
@@ -150,30 +150,32 @@ pub fn set_color(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -
     // reset all colors/attributes. Same if foreground is "reset" (undocumented).
     if is_reset || [fg, bg].iter().any(|c| c.is_some_and(|c| c.is_normal())) {
         outp.reset_text_face();
-    } else {
-        // Historically we have not used set_text_face() for colors here.
-        // Doing so would add two magic behaviors:
-        // - if fg and bg are equal, it makes one of them white
-        // - if bg is not normal, it makes the foreground bold
-        // The first one seems fine but the second one not really.
-        outp.set_text_face(TextFace::new(Color::None, Color::None, Color::None, style));
-        if let Some(fg) = fg {
-            if !outp.write_color(Paintable::Foreground, fg) {
-                // We need to do *something* or the lack of any output messes up
-                // when the cartesian product here would make "foo" disappear:
-                //  $ echo (set_color foo)bar
-                outp.reset_text_face();
-            }
+    }
+
+    // Historically we have not used set_text_face() for colors here.
+    // Doing so would add two magic behaviors:
+    // - if fg and bg are equal, it makes one of them white
+    // - if bg is not normal, it makes the foreground bold
+    // The first one seems fine but the second one not really.
+    outp.set_text_face(TextFace::new(Color::None, Color::None, Color::None, style));
+    if let Some(fg) = fg {
+        if !fg.is_normal() && !outp.write_color(Paintable::Foreground, fg) {
+            // We need to do *something* or the lack of any output messes up
+            // when the cartesian product here would make "foo" disappear:
+            //  $ echo (set_color foo)bar
+            outp.reset_text_face();
         }
-        if let Some(bg) = bg {
+    }
+    if let Some(bg) = bg {
+        if !bg.is_normal() {
             outp.write_color(Paintable::Background, bg);
         }
-        if let Some(underline_color) = underline_color {
-            if underline_color.is_normal() {
-                outp.write_command(DefaultUnderlineColor);
-            } else {
-                outp.write_color(Paintable::Underline, underline_color);
-            }
+    }
+    if let Some(underline_color) = underline_color {
+        if underline_color.is_normal() {
+            outp.write_command(DefaultUnderlineColor);
+        } else {
+            outp.write_color(Paintable::Underline, underline_color);
         }
     }
 
diff --git a/tests/checks/set_color.fish b/tests/checks/set_color.fish
index ccdf92988..62d10c5e0 100644
--- a/tests/checks/set_color.fish
+++ b/tests/checks/set_color.fish
@@ -14,7 +14,7 @@ string escape (set_color --background=reset)
 # CHECKERR: set_color: Unknown color 'reset'
 
 string escape (set_color --bold red --background=normal)
-# CHECK: \e\[m
+# CHECK: \e\[m\e\[1m\e\[31m
 string escape (set_color --bold red --background=blue)
 # CHECK: \e\[1m\e\[31m\e\[44m