diff --git a/src/builtins/set_color.rs b/src/builtins/set_color.rs index dfa0eaaa2..c6500f97c 100644 --- a/src/builtins/set_color.rs +++ b/src/builtins/set_color.rs @@ -3,8 +3,8 @@ use super::prelude::*; use crate::color::Color; use crate::common::str2wcstring; -use crate::terminal::TerminalCommand::{DefaultBackgroundColor, DefaultUnderlineColor}; -use crate::terminal::{Output, Outputter, Paintable}; +use crate::screen::is_dumb; +use crate::terminal::{use_terminfo, Outputter}; use crate::text_face::{ self, parse_text_face_and_options, PrintColorsArgs, SpecifiedTextFace, TextFace, TextStyling, }; @@ -112,46 +112,27 @@ pub fn set_color(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) - } }; - let mut outp = Outputter::new_buffering(); + let mut outp = Outputter::new_buffering_no_assume_normal(); - let fg = specified_face.fg; - let bg = specified_face.bg; - let underline_color = specified_face.underline_color; - let style = specified_face.style; - - // Here's some automagic behavior: if either of foreground or background are "normal", - // reset all colors/attributes. Same if foreground is "reset" (undocumented). - if is_reset || fg.is_some_and(|fg| fg.is_normal()) { + // Note that for historical reasons "set_color normal" reset all colors/attributes. So does + // "set_color reset" (undocumented). + if is_reset { 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 bg.is_normal() { - outp.write_command(DefaultBackgroundColor); - } else { - 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); - } + outp.set_text_face_no_magic(TextFace::new( + specified_face.fg.unwrap_or(Color::None), + specified_face.bg.unwrap_or(Color::None), + specified_face.underline_color.unwrap_or(Color::None), + specified_face.style, + )); + + if specified_face.fg.is_some() && outp.contents().is_empty() { + assert!(is_dumb() || use_terminfo()); + // 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(); } // Output the collected string. diff --git a/src/terminal.rs b/src/terminal.rs index 26b0ed9f8..45a5317ee 100644 --- a/src/terminal.rs +++ b/src/terminal.rs @@ -444,6 +444,14 @@ pub fn new_buffering() -> Self { Self::new_from_fd(-1) } + pub fn new_buffering_no_assume_normal() -> Self { + let mut zelf = Self::new_buffering(); + zelf.last.fg = Color::None; + zelf.last.bg = Color::None; + assert_eq!(zelf.last.underline_color, Color::None); + zelf + } + fn maybe_flush(&mut self) { if self.fd >= 0 && self.buffer_count == 0 { self.flush_to(self.fd); @@ -494,8 +502,14 @@ pub(crate) fn reset_text_face(&mut self) { /// /// - Lastly we may need to write set_a_background or set_a_foreground to set the other half of the /// color pair to what it should be. - #[allow(clippy::if_same_then_else)] pub(crate) fn set_text_face(&mut self, face: TextFace) { + self.set_text_face_internal(face, true) + } + pub(crate) fn set_text_face_no_magic(&mut self, face: TextFace) { + self.set_text_face_internal(face, false) + } + #[allow(clippy::if_same_then_else)] + fn set_text_face_internal(&mut self, face: TextFace, salvage_unreadable: bool) { let mut fg = face.fg; let bg = face.bg; let underline_color = face.underline_color; @@ -519,12 +533,14 @@ pub(crate) fn set_text_face(&mut self, face: TextFace) { // Only way to exit non-resettable ones is a reset of all attributes. self.reset_text_face(); } - if !bg.is_special() && fg == bg { - fg = if bg == Color::WHITE { - Color::BLACK - } else { - Color::WHITE - }; + if salvage_unreadable { + if !bg.is_special() && fg == bg { + fg = if bg == Color::WHITE { + Color::BLACK + } else { + Color::WHITE + }; + } } if !fg.is_none() && fg != self.last.fg { diff --git a/tests/checks/set_color.fish b/tests/checks/set_color.fish index 7e7ad28ec..9c90024ba 100644 --- a/tests/checks/set_color.fish +++ b/tests/checks/set_color.fish @@ -14,9 +14,9 @@ string escape (set_color --background=reset) # CHECKERR: set_color: Unknown color 'reset' string escape (set_color --bold red --background=normal) -# CHECK: \e\[1m\e\[31m\e\[49m +# CHECK: \e\[31m\e\[49m\e\[1m string escape (set_color --bold red --background=blue) -# CHECK: \e\[1m\e\[31m\e\[44m +# CHECK: \e\[31m\e\[44m\e\[1m string escape (set_color --background=f00 --background=green --background=00f) # CHECK: \e\[48\;2\;255\;0\;0m diff --git a/tests/checks/string.fish b/tests/checks/string.fish index cb91753ff..c64ab789b 100644 --- a/tests/checks/string.fish +++ b/tests/checks/string.fish @@ -1045,12 +1045,12 @@ end string shorten -m6 (set_color blue)s(set_color red)t(set_color --bold brwhite)rin(set_color red)g(set_color yellow)-shorten | string escape # Renders like "strin…" in colors # Note that red sequence that we still pass on because it's width 0. -# CHECK: \e\[34ms\e\[31mt\e\[1m\e\[97mrin\e\[31m… +# CHECK: \e\[34ms\e\[31mt\e\[97m\e\[1mrin\e\[31m… # See that colors aren't counted in ellipsis string shorten -c (set_color blue)s(set_color red)t(set_color --bold brwhite)rin(set_color red)g -m 8 abcdefghijklmno | string escape # Renders like "abstring" in colors -# CHECK: ab\e\[34ms\e\[31mt\e\[1m\e\[97mrin\e\[31mg +# CHECK: ab\e\[34ms\e\[31mt\e\[97m\e\[1mrin\e\[31mg set -l str (set_color blue)s(set_color red)t(set_color --bold brwhite)rin(set_color red)g(set_color yellow)-shorten for i in (seq 1 (string length -V -- $str))