From 6e8c32eb12d8f7bc86ce4239d92a1512ef3248d5 Mon Sep 17 00:00:00 2001 From: Nahor Date: Tue, 10 Mar 2026 11:17:21 -0700 Subject: [PATCH] Remove the `Default` trait for text face/styling The notion of default is context dependent: there is the default for the state (default color, non-bold, no underline, ...) and the default for a change (no color change, no underline change, ...). Currently, using a single default works because either the style attributes cannot be turned off individually (the user is expected to reset to default then re-set necessary attributes), or the code has special handling for specific scenarios (e.g. highlighting). So in preparation for later commits, where attribute can be turned off individually, make the two defaults explicit. Part of #12507 --- src/builtins/set_color.rs | 4 ++-- src/highlight/highlight.rs | 16 ++++++++-------- src/reader/reader.rs | 5 +++-- src/terminal.rs | 17 ++++++----------- src/text_face.rs | 37 +++++++++++++++++++++++++------------ 5 files changed, 44 insertions(+), 35 deletions(-) diff --git a/src/builtins/set_color.rs b/src/builtins/set_color.rs index 50ded0496..e32cfd252 100644 --- a/src/builtins/set_color.rs +++ b/src/builtins/set_color.rs @@ -34,7 +34,7 @@ fn print_colors( fg, bg.unwrap_or(Color::None), underline_color.unwrap_or(Color::None), - style.unwrap_or_default(), + style.unwrap_or(TextStyling::unknown()), )); } outp.write_wstr(color_name); @@ -131,7 +131,7 @@ pub fn set_color(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) - 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.unwrap_or_default(), + specified_face.style.unwrap_or(TextStyling::unknown()), ), with_reset, ); diff --git a/src/highlight/highlight.rs b/src/highlight/highlight.rs index 3265b18c1..730dfe457 100644 --- a/src/highlight/highlight.rs +++ b/src/highlight/highlight.rs @@ -24,7 +24,7 @@ }; use crate::path::{path_as_implicit_cd, path_get_cdpath, path_get_path, paths_are_same_file}; use crate::terminal::Outputter; -use crate::text_face::{SpecifiedTextFace, TextFace, UnderlineStyle, parse_text_face}; +use crate::text_face::{SpecifiedTextFace, TextFace, TextStyling, UnderlineStyle, parse_text_face}; use crate::threads::assert_is_background_thread; use crate::tokenizer::{PipeOrRedir, variable_assignment_equals_pos}; use fish_color::Color; @@ -75,11 +75,11 @@ pub fn colorize(text: &wstr, colors: &[HighlightSpec], vars: &dyn Environment) - last_color = color; } if i + 1 == text.char_count() && c == '\n' { - outp.set_text_face(TextFace::default()); + outp.set_text_face(TextFace::terminal_default()); } outp.writech(c); } - outp.set_text_face(TextFace::default()); + outp.set_text_face(TextFace::terminal_default()); outp.contents().to_owned() } @@ -169,7 +169,7 @@ fn resolve_spec_uncached(highlight: &HighlightSpec, vars: &dyn Environment) -> T return face; } } - TextFace::default() + TextFace::terminal_default() }; let mut face = resolve_role(highlight.foreground); @@ -186,8 +186,8 @@ fn resolve_spec_uncached(highlight: &HighlightSpec, vars: &dyn Environment) -> T if highlight.valid_path { if let Some(valid_path_var) = vars.get(L!("fish_color_valid_path")) { // Historical behavior is to not apply background. - let valid_path_face = - parse_text_face_for_highlight(&valid_path_var).unwrap_or_default(); + let valid_path_face = parse_text_face_for_highlight(&valid_path_var) + .unwrap_or(TextFace::terminal_default()); // Apply the foreground, except if it's normal. The intention here is likely // to only override foreground if the valid path color has an explicit foreground. if !valid_path_face.fg.is_normal() { @@ -209,11 +209,11 @@ fn resolve_spec_uncached(highlight: &HighlightSpec, vars: &dyn Environment) -> T pub(crate) fn parse_text_face_for_highlight(var: &EnvVar) -> Option { let face = parse_text_face(var.as_list()); (face != SpecifiedTextFace::default()).then(|| { - let default = TextFace::default(); + let default = TextFace::terminal_default(); let fg = face.fg.unwrap_or(default.fg); let bg = face.bg.unwrap_or(default.bg); let underline_color = face.underline_color.unwrap_or(default.underline_color); - let style = face.style.unwrap_or_default(); + let style = face.style.unwrap_or(TextStyling::terminal_default()); TextFace { fg, bg, diff --git a/src/reader/reader.rs b/src/reader/reader.rs index a2916d3c1..a3661b1e7 100644 --- a/src/reader/reader.rs +++ b/src/reader/reader.rs @@ -3099,7 +3099,8 @@ fn handle_readline_command(&mut self, c: ReadlineCmd) { let mut outp = Outputter::stdoutput().borrow_mut(); if let Some(fish_color_cancel) = self.vars().get(L!("fish_color_cancel")) { outp.set_text_face( - parse_text_face_for_highlight(&fish_color_cancel).unwrap_or_default(), + parse_text_face_for_highlight(&fish_color_cancel) + .unwrap_or(TextFace::terminal_default()), ); } outp.write_wstr(L!("^C")); @@ -6339,7 +6340,7 @@ fn reader_run_command(parser: &Parser, cmd: &wstr) -> EvalRes { reader_write_title(cmd, parser, true); Outputter::stdoutput() .borrow_mut() - .set_text_face(TextFace::default()); + .set_text_face(TextFace::terminal_default()); term_donate(false); let time_before = Instant::now(); diff --git a/src/terminal.rs b/src/terminal.rs index 3d4dae936..68b6e5eb5 100644 --- a/src/terminal.rs +++ b/src/terminal.rs @@ -255,7 +255,7 @@ const fn new_from_fd(fd: RawFd) -> Self { contents: Vec::new(), buffer_count: 0, fd, - last: TextFace::default(), + last: TextFace::terminal_default(), } } @@ -266,8 +266,7 @@ pub fn new_buffering() -> Self { pub fn new_buffering_no_assume_normal() -> Self { let mut zelf = Self::new_buffering(); - zelf.last.fg = Color::None; - zelf.last.bg = Color::None; + zelf.last = TextFace::unknown(); assert_eq!(zelf.last.underline_color, Color::None); zelf } @@ -378,7 +377,7 @@ fn set_text_face_internal( use SgrTerminalCommand::{ DefaultBackgroundColor, DefaultUnderlineColor, EnterBoldMode, EnterDimMode, EnterItalicsMode, EnterReverseMode, EnterStrikethroughMode, EnterUnderlineMode, - ExitAttributeMode, ExitItalicsMode, ExitStrikethroughMode, ExitUnderlineMode, + ExitItalicsMode, ExitStrikethroughMode, ExitUnderlineMode, }; let mut style_writer = self.style_writer(); @@ -409,16 +408,12 @@ fn set_text_face_internal( if !fg.is_none() && fg != style_writer.last().fg { if fg.is_normal() { - style_writer.write_command(ExitAttributeMode); - - style_writer.last().bg = Color::Normal; - style_writer.last().underline_color = Color::Normal; - style_writer.last().style = TextStyling::default(); + style_writer.reset_text_face(); } else { assert!(!fg.is_special()); style_writer.write_color(Paintable::Foreground, fg); + style_writer.last().fg = fg; } - style_writer.last().fg = fg; } if !bg.is_none() && bg != style_writer.last().bg { @@ -644,7 +639,7 @@ fn last(&mut self) -> &mut TextFace { pub(crate) fn reset_text_face(&mut self) { use SgrTerminalCommand::ExitAttributeMode; self.write_command(ExitAttributeMode); - self.out.last = TextFace::default(); + self.out.last = TextFace::terminal_default(); } pub(crate) fn write_command(&mut self, cmd: SgrTerminalCommand) -> bool { diff --git a/src/text_face.rs b/src/text_face.rs index 7f3853b87..3ce084395 100644 --- a/src/text_face.rs +++ b/src/text_face.rs @@ -30,7 +30,7 @@ fn difference_prefer_empty(self, other: Self) -> Self { } } -#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] +#[derive(Copy, Clone, Debug, PartialEq, Eq)] pub(crate) struct TextStyling { pub(crate) bold: bool, pub(crate) underline_style: Option, @@ -41,7 +41,7 @@ pub(crate) struct TextStyling { } impl TextStyling { - pub(crate) const fn default() -> Self { + pub(crate) const fn terminal_default() -> Self { Self { bold: false, underline_style: None, @@ -51,8 +51,19 @@ pub(crate) const fn default() -> Self { strikethrough: false, } } + pub(crate) const fn unknown() -> Self { + Self { + bold: false, + underline_style: None, + italics: false, + dim: false, + reverse: false, + strikethrough: false, + } + } + pub(crate) fn is_empty(&self) -> bool { - *self == Self::default() + *self == Self::unknown() } pub(crate) fn union_prefer_right(self, other: Self) -> Self { Self { @@ -123,19 +134,21 @@ pub(crate) struct TextFace { pub(crate) style: TextStyling, } -impl Default for TextFace { - fn default() -> Self { - Self::default() - } -} - impl TextFace { - pub const fn default() -> Self { + pub const fn terminal_default() -> Self { Self { fg: Color::Normal, bg: Color::Normal, + underline_color: Color::Normal, + style: TextStyling::terminal_default(), + } + } + pub const fn unknown() -> Self { + Self { + fg: Color::None, + bg: Color::None, underline_color: Color::None, - style: TextStyling::default(), + style: TextStyling::unknown(), } } @@ -232,7 +245,7 @@ pub(crate) fn parse_text_face_and_options<'argarray, 'args>( let mut underline_colors = vec![]; let mut style: Option = None; fn init_style(style: &mut Option) -> &mut TextStyling { - style.get_or_insert_default() + style.get_or_insert(TextStyling::unknown()) } let mut print_color_mode = false;