From 939902eb29177b89aa2fbe017f6b5654eb9b5e18 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Mon, 21 Apr 2025 16:08:51 +0200 Subject: [PATCH] Remove mysterious workaround for buggy sgr0 handling This workaround was added in commit d66700a0e4d (Color work, 2012-02-11). I don't understand why it would be necessary, it seems redundant. Bravely remove it. Would be interesting to know which terminal caused the motivating problem. Terminal.app and others seem to be unaffected by this change. --- src/builtins/set_color.rs | 6 +++--- src/reader.rs | 8 ++++---- src/terminal.rs | 14 +++----------- 3 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/builtins/set_color.rs b/src/builtins/set_color.rs index ab270fd12..3f690cd46 100644 --- a/src/builtins/set_color.rs +++ b/src/builtins/set_color.rs @@ -30,7 +30,7 @@ fn print_colors(streams: &mut IoStreams, args: &[&wstr], style: TextStyling, bg: if streams.out_is_terminal() && bg.is_some() { // If we have a background, stop it after the color // or it goes to the end of the line and looks ugly. - outp.reset_text_face(false); + outp.reset_text_face(); } outp.writech('\n'); } // conveniently, 'normal' is always the last color so we don't need to reset here @@ -124,7 +124,7 @@ pub fn set_color(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) - // reset all colors/attributes. Same if foreground is "reset" (undocumented). // Note that either overwrite the attributes printed above! For "normal", this is probably wrong? if is_reset || [fg, bg].iter().any(|c| c.is_some_and(|c| c.is_normal())) { - outp.reset_text_face(false); + outp.reset_text_face(); } else { outp.set_text_face(TextFace::new(Color::None, Color::None, style)); if let Some(fg) = fg { @@ -132,7 +132,7 @@ pub fn set_color(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) - // 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(true); + outp.reset_text_face(); } } if let Some(bg) = bg { diff --git a/src/reader.rs b/src/reader.rs index df2f2d843..fcccdaf18 100644 --- a/src/reader.rs +++ b/src/reader.rs @@ -2290,7 +2290,7 @@ fn readline(&mut self, nchars: Option) -> Option { } perror("tcsetattr"); // return to previous mode } - Outputter::stdoutput().borrow_mut().reset_text_face(true); + Outputter::stdoutput().borrow_mut().reset_text_face(); } let result = self .rls() @@ -2695,7 +2695,7 @@ fn handle_readline_command(&mut self, c: ReadlineCmd) { outp.set_text_face(parse_text_face_for_highlight(&fish_color_cancel)); } outp.write_wstr(L!("^C")); - outp.reset_text_face(true); + outp.reset_text_face(); // We print a newline last so the prompt_sp hack doesn't get us. outp.push(b'\n'); @@ -4459,7 +4459,7 @@ fn reader_interactive_init(parser: &Parser) { /// Destroy data for interactive use. fn reader_interactive_destroy() { - Outputter::stdoutput().borrow_mut().reset_text_face(true); + Outputter::stdoutput().borrow_mut().reset_text_face(); } /// Return whether fish is currently unwinding the stack in preparation to exit. @@ -4523,7 +4523,7 @@ pub fn reader_write_title( out.write_command(Osc0WindowTitle(&lst)); } - out.reset_text_face(true); + out.reset_text_face(); if reset_cursor_position && !lst.is_empty() { // Put the cursor back at the beginning of the line (issue #2453). out.write_bytes(b"\r"); diff --git a/src/terminal.rs b/src/terminal.rs index 017e5c808..0f5f59d1b 100644 --- a/src/terminal.rs +++ b/src/terminal.rs @@ -466,12 +466,7 @@ pub fn write_color(&mut self, color: Color, is_fg: bool) -> bool { } /// Unconditionally resets colors and text style. - pub(crate) fn reset_text_face(&mut self, weird_workaround: bool) { - if weird_workaround { - // If we exit attribute mode, we must first set a color, or previously colored text might - // lose its color. Terminals are weird... - write_foreground_color(self, 0); - } + pub(crate) fn reset_text_face(&mut self) { use TerminalCommand::ExitAttributeMode; self.write_command(ExitAttributeMode); self.last = TextFace::default(); @@ -516,7 +511,7 @@ pub(crate) fn set_text_face(&mut self, face: TextFace) { non_resettable(self.last.style).difference(non_resettable(style)); if !non_resettable_attributes_to_unset.is_empty() { // Only way to exit non-resettable ones is a reset of all attributes. - self.reset_text_face(false); + self.reset_text_face(); } if !self.last.bg.is_special() { // Background was set. @@ -543,12 +538,11 @@ pub(crate) fn set_text_face(&mut self, face: TextFace) { } if !bg_set && last_bg_set { // Background color changed and is no longer set, so we exit bold mode. - self.reset_text_face(false); + self.reset_text_face(); } if !fg.is_none() && fg != self.last.fg { if fg.is_normal() { - write_foreground_color(self, 0); self.write_command(ExitAttributeMode); self.last.bg = Color::Normal; @@ -562,8 +556,6 @@ pub(crate) fn set_text_face(&mut self, face: TextFace) { if !bg.is_none() && bg != self.last.bg { if bg.is_normal() { - write_background_color(self, 0); - self.write_command(ExitAttributeMode); if !self.last.fg.is_normal() { self.write_color(self.last.fg, true /* foreground */);