From 5b9ba559c76a7ee43ce1e634669c3f3cfffaee4e Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sat, 12 Apr 2025 16:22:21 +0200 Subject: [PATCH] Rename RgbColor to Color RgbColor can also be a named color, so this name is surprising to readers who don't have the implicit assumption that named colors are the default and that RGB support is something novel. --- src/builtins/set_color.rs | 20 +++++------ src/color.rs | 72 +++++++++++++++++++------------------- src/highlight/highlight.rs | 16 ++++----- src/reader.rs | 12 +++---- src/terminal.rs | 46 ++++++++++++------------ 5 files changed, 83 insertions(+), 83 deletions(-) diff --git a/src/builtins/set_color.rs b/src/builtins/set_color.rs index 4def1fab8..d43245f39 100644 --- a/src/builtins/set_color.rs +++ b/src/builtins/set_color.rs @@ -1,7 +1,7 @@ // Implementation of the set_color builtin. use super::prelude::*; -use crate::color::RgbColor; +use crate::color::Color; use crate::common::str2wcstring; use crate::terminal::TerminalCommand::{ EnterBoldMode, EnterDimMode, EnterItalicsMode, EnterReverseMode, EnterStandoutMode, @@ -17,7 +17,7 @@ fn print_modifiers( italics: bool, dim: bool, reverse: bool, - bg: RgbColor, + bg: Color, ) { if bold { outp.write_command(EnterBoldMode); @@ -55,7 +55,7 @@ fn print_colors( italics: bool, dim: bool, reverse: bool, - bg: RgbColor, + bg: Color, ) { let outp = &mut Outputter::new_buffering(); @@ -64,15 +64,15 @@ fn print_colors( let args = if !args.is_empty() { args } else { - named_colors = RgbColor::named_color_names(); + named_colors = Color::named_color_names(); &named_colors }; for color_name in args { if streams.out_is_terminal() { print_modifiers(outp, bold, underline, italics, dim, reverse, bg); - let color = RgbColor::from_wstr(color_name).unwrap_or(RgbColor::NONE); - outp.set_color(color, RgbColor::NONE); + let color = Color::from_wstr(color_name).unwrap_or(Color::NONE); + outp.set_color(color, Color::NONE); if !bg.is_none() { outp.write_color(bg, false /* not is_fg */); } @@ -159,7 +159,7 @@ pub fn set_color(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) - // We want to reclaim argv so grab wopt_index now. let mut wopt_index = w.wopt_index; - let mut bg = RgbColor::from_wstr(bgcolor.unwrap_or(L!(""))).unwrap_or(RgbColor::NONE); + let mut bg = Color::from_wstr(bgcolor.unwrap_or(L!(""))).unwrap_or(Color::NONE); if bgcolor.is_some() && bg.is_none() { streams.err.append(wgettext_fmt!( "%ls: Unknown color '%ls'\n", @@ -174,7 +174,7 @@ pub fn set_color(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) - // for --print-colors. Because it's not interesting in terms of display, // just skip it. if bgcolor.is_some() && bg.is_special() { - bg = RgbColor::from_wstr(L!("")).unwrap_or(RgbColor::NONE); + bg = Color::from_wstr(L!("")).unwrap_or(Color::NONE); } let args = &argv[wopt_index..argc]; print_colors(streams, args, bold, underline, italics, dim, reverse, bg); @@ -184,7 +184,7 @@ pub fn set_color(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) - // Remaining arguments are foreground color. let mut fgcolors = Vec::new(); while wopt_index < argc { - let fg = RgbColor::from_wstr(argv[wopt_index]).unwrap_or(RgbColor::NONE); + let fg = Color::from_wstr(argv[wopt_index]).unwrap_or(Color::NONE); if fg.is_none() { streams.err.append(wgettext_fmt!( "%ls: Unknown color '%ls'\n", @@ -215,7 +215,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.set_color(RgbColor::RESET, RgbColor::NONE); + outp.set_color(Color::RESET, Color::NONE); } } if bgcolor.is_some() && !bg.is_normal() && !bg.is_reset() { diff --git a/src/color.rs b/src/color.rs index b685296cb..7b7db1f52 100644 --- a/src/color.rs +++ b/src/color.rs @@ -46,12 +46,12 @@ pub struct Flags: u8 { /// A type that represents a color. #[derive(Debug, Copy, Clone, PartialEq, Eq)] -pub struct RgbColor { +pub struct Color { pub typ: Type, pub flags: Flags, } -impl RgbColor { +impl Color { /// The color white pub const WHITE: Self = Self { typ: Type::Named { idx: 7 }, @@ -293,7 +293,7 @@ fn try_parse_rgb(mut s: &wstr) -> Option { } else { return None; }; - Some(RgbColor::from_rgb(r, g, b)) + Some(Color::from_rgb(r, g, b)) } /// Try parsing an explicit color name like "magenta". @@ -436,37 +436,37 @@ fn term256_color_for_rgb(color: Color24) -> u8 { #[cfg(test)] mod tests { - use crate::color::{Color24, Flags, RgbColor, Type}; + use crate::color::{Color, Color24, Flags, Type}; use crate::wchar::prelude::*; #[test] fn parse() { - assert!(RgbColor::from_wstr(L!("#FF00A0")).unwrap().is_rgb()); - assert!(RgbColor::from_wstr(L!("FF00A0")).unwrap().is_rgb()); - assert!(RgbColor::from_wstr(L!("#F30")).unwrap().is_rgb()); - assert!(RgbColor::from_wstr(L!("F30")).unwrap().is_rgb()); - assert!(RgbColor::from_wstr(L!("f30")).unwrap().is_rgb()); - assert!(RgbColor::from_wstr(L!("#FF30a5")).unwrap().is_rgb()); - assert!(RgbColor::from_wstr(L!("3f30")).is_none()); - assert!(RgbColor::from_wstr(L!("##f30")).is_none()); - assert!(RgbColor::from_wstr(L!("magenta")).unwrap().is_named()); - assert!(RgbColor::from_wstr(L!("MaGeNTa")).unwrap().is_named()); - assert!(RgbColor::from_wstr(L!("mooganta")).is_none()); + assert!(Color::from_wstr(L!("#FF00A0")).unwrap().is_rgb()); + assert!(Color::from_wstr(L!("FF00A0")).unwrap().is_rgb()); + assert!(Color::from_wstr(L!("#F30")).unwrap().is_rgb()); + assert!(Color::from_wstr(L!("F30")).unwrap().is_rgb()); + assert!(Color::from_wstr(L!("f30")).unwrap().is_rgb()); + assert!(Color::from_wstr(L!("#FF30a5")).unwrap().is_rgb()); + assert!(Color::from_wstr(L!("3f30")).is_none()); + assert!(Color::from_wstr(L!("##f30")).is_none()); + assert!(Color::from_wstr(L!("magenta")).unwrap().is_named()); + assert!(Color::from_wstr(L!("MaGeNTa")).unwrap().is_named()); + assert!(Color::from_wstr(L!("mooganta")).is_none()); } #[test] fn parse_rgb() { - assert!(RgbColor::from_wstr(L!("##FF00A0")).is_none()); - assert!(RgbColor::from_wstr(L!("#FF00A0")) == Some(RgbColor::from_rgb(0xff, 0x00, 0xa0))); - assert!(RgbColor::from_wstr(L!("FF00A0")) == Some(RgbColor::from_rgb(0xff, 0x00, 0xa0))); - assert!(RgbColor::from_wstr(L!("FAF")) == Some(RgbColor::from_rgb(0xff, 0xaa, 0xff))); + assert!(Color::from_wstr(L!("##FF00A0")).is_none()); + assert!(Color::from_wstr(L!("#FF00A0")) == Some(Color::from_rgb(0xff, 0x00, 0xa0))); + assert!(Color::from_wstr(L!("FF00A0")) == Some(Color::from_rgb(0xff, 0x00, 0xa0))); + assert!(Color::from_wstr(L!("FAF")) == Some(Color::from_rgb(0xff, 0xaa, 0xff))); } // Regression test for multiplicative overflow in convert_color. #[test] fn test_term16_color_for_rgb() { for c in 0..=u8::MAX { - let color = RgbColor { + let color = Color { typ: Type::Rgb(Color24 { r: c, g: c, b: c }), flags: Flags::DEFAULT, }; @@ -477,52 +477,52 @@ fn test_term16_color_for_rgb() { #[test] fn parse_short_hex_with_hash() { assert_eq!( - RgbColor::try_parse_rgb(L!("#F3A")), - Some(RgbColor::from_rgb(0xFF, 0x33, 0xAA)) + Color::try_parse_rgb(L!("#F3A")), + Some(Color::from_rgb(0xFF, 0x33, 0xAA)) ); } #[test] fn parse_long_hex_with_hash() { assert_eq!( - RgbColor::try_parse_rgb(L!("#F3A035")), - Some(RgbColor::from_rgb(0xF3, 0xA0, 0x35)) + Color::try_parse_rgb(L!("#F3A035")), + Some(Color::from_rgb(0xF3, 0xA0, 0x35)) ); } #[test] fn parse_short_hex_without_hash() { assert_eq!( - RgbColor::try_parse_rgb(L!("F3A")), - Some(RgbColor::from_rgb(0xFF, 0x33, 0xAA)) + Color::try_parse_rgb(L!("F3A")), + Some(Color::from_rgb(0xFF, 0x33, 0xAA)) ); } #[test] fn parse_long_hex_without_hash() { assert_eq!( - RgbColor::try_parse_rgb(L!("F3A035")), - Some(RgbColor::from_rgb(0xF3, 0xA0, 0x35)) + Color::try_parse_rgb(L!("F3A035")), + Some(Color::from_rgb(0xF3, 0xA0, 0x35)) ); } #[test] fn invalid_hex_length() { - assert_eq!(RgbColor::try_parse_rgb(L!("#F3A03")), None); - assert_eq!(RgbColor::try_parse_rgb(L!("F3A0")), None); + assert_eq!(Color::try_parse_rgb(L!("#F3A03")), None); + assert_eq!(Color::try_parse_rgb(L!("F3A0")), None); } #[test] fn invalid_hex_character() { - assert_eq!(RgbColor::try_parse_rgb(L!("#GFA")), None); - assert_eq!(RgbColor::try_parse_rgb(L!("F3G035")), None); + assert_eq!(Color::try_parse_rgb(L!("#GFA")), None); + assert_eq!(Color::try_parse_rgb(L!("F3G035")), None); } #[test] fn invalid_hash_combinations() { - assert_eq!(RgbColor::try_parse_rgb(L!("##F3A")), None); - assert_eq!(RgbColor::try_parse_rgb(L!("###F3A035")), None); - assert_eq!(RgbColor::try_parse_rgb(L!("F3A#")), None); - assert_eq!(RgbColor::try_parse_rgb(L!("#F#3A")), None); + assert_eq!(Color::try_parse_rgb(L!("##F3A")), None); + assert_eq!(Color::try_parse_rgb(L!("###F3A035")), None); + assert_eq!(Color::try_parse_rgb(L!("F3A#")), None); + assert_eq!(Color::try_parse_rgb(L!("#F#3A")), None); } } diff --git a/src/highlight/highlight.rs b/src/highlight/highlight.rs index 8cdcb15e9..a04a710ec 100644 --- a/src/highlight/highlight.rs +++ b/src/highlight/highlight.rs @@ -6,7 +6,7 @@ VariableAssignment, }; use crate::builtins::shared::builtin_exists; -use crate::color::RgbColor; +use crate::color::Color; use crate::common::{ valid_var_name, valid_var_name_char, ASCII_MAX, EXPAND_RESERVED_BASE, EXPAND_RESERVED_END, }; @@ -71,12 +71,12 @@ pub fn colorize(text: &wstr, colors: &[HighlightSpec], vars: &dyn Environment) - for (i, c) in text.chars().enumerate() { let color = colors[i]; if color != last_color { - outp.set_color(rv.resolve_spec(&color, false, vars), RgbColor::NORMAL); + outp.set_color(rv.resolve_spec(&color, false, vars), Color::NORMAL); last_color = color; } outp.writech(c); } - outp.set_color(RgbColor::NORMAL, RgbColor::NORMAL); + outp.set_color(Color::NORMAL, Color::NORMAL); outp.contents().to_owned() } @@ -107,8 +107,8 @@ pub fn highlight_shell( /// one screen redraw. #[derive(Default)] pub struct HighlightColorResolver { - fg_cache: HashMap, - bg_cache: HashMap, + fg_cache: HashMap, + bg_cache: HashMap, } /// highlight_color_resolver_t resolves highlight specs (like "a command") to actual RGB colors. @@ -124,7 +124,7 @@ pub fn resolve_spec( highlight: &HighlightSpec, is_background: bool, vars: &dyn Environment, - ) -> RgbColor { + ) -> Color { let cache = if is_background { &mut self.bg_cache } else { @@ -143,8 +143,8 @@ pub fn resolve_spec_uncached( highlight: &HighlightSpec, is_background: bool, vars: &dyn Environment, - ) -> RgbColor { - let mut result = RgbColor::NORMAL; + ) -> Color { + let mut result = Color::NORMAL; let role = if is_background { highlight.background } else { diff --git a/src/reader.rs b/src/reader.rs index 1b7f321ad..d598b03e2 100644 --- a/src/reader.rs +++ b/src/reader.rs @@ -46,7 +46,7 @@ use crate::builtins::shared::ErrorCode; use crate::builtins::shared::STATUS_CMD_ERROR; use crate::builtins::shared::STATUS_CMD_OK; -use crate::color::RgbColor; +use crate::color::Color; use crate::common::restore_term_foreground_process_group_for_exit; use crate::common::{ escape, escape_string, exit_without_destructors, get_ellipsis_char, get_obfuscation_read_char, @@ -2292,7 +2292,7 @@ fn readline(&mut self, nchars: Option) -> Option { } Outputter::stdoutput() .borrow_mut() - .set_color(RgbColor::RESET, RgbColor::RESET); + .set_color(Color::RESET, Color::RESET); } let result = self .rls() @@ -2700,7 +2700,7 @@ fn handle_readline_command(&mut self, c: ReadlineCmd) { ); } outp.write_wstr(L!("^C")); - outp.set_color(RgbColor::RESET, RgbColor::RESET); + outp.set_color(Color::RESET, Color::RESET); // We print a newline last so the prompt_sp hack doesn't get us. outp.push(b'\n'); @@ -4466,7 +4466,7 @@ fn reader_interactive_init(parser: &Parser) { fn reader_interactive_destroy() { Outputter::stdoutput() .borrow_mut() - .set_color(RgbColor::RESET, RgbColor::RESET); + .set_color(Color::RESET, Color::RESET); } /// Return whether fish is currently unwinding the stack in preparation to exit. @@ -4530,7 +4530,7 @@ pub fn reader_write_title( out.write_command(Osc0WindowTitle(&lst)); } - out.set_color(RgbColor::RESET, RgbColor::RESET); + out.set_color(Color::RESET, Color::RESET); if reset_cursor_position && !lst.is_empty() { // Put the cursor back at the beginning of the line (issue #2453). out.write_bytes(b"\r"); @@ -5659,7 +5659,7 @@ fn reader_run_command(parser: &Parser, cmd: &wstr) -> EvalRes { reader_write_title(cmd, parser, true); Outputter::stdoutput() .borrow_mut() - .set_color(RgbColor::NORMAL, RgbColor::NORMAL); + .set_color(Color::NORMAL, Color::NORMAL); term_donate(false); let time_before = Instant::now(); diff --git a/src/terminal.rs b/src/terminal.rs index 9761349d7..62dbab861 100644 --- a/src/terminal.rs +++ b/src/terminal.rs @@ -1,5 +1,5 @@ // Generic output functions. -use crate::color::{self, Color24, RgbColor}; +use crate::color::{self, Color, Color24}; use crate::common::ToCString; use crate::common::{self, escape_string, wcs2string, wcs2string_appending, EscapeStringStyle}; use crate::env::EnvVar; @@ -232,7 +232,7 @@ pub(crate) fn use_terminfo() -> bool { fn palette_color(out: &mut impl Output, foreground: bool, mut idx: u8) -> bool { if only_grayscale() - && !(RgbColor { + && !(Color { typ: color::Type::Named { idx }, flags: color::Flags::DEFAULT, }) @@ -395,7 +395,7 @@ fn scroll_forward(out: &mut impl Output, lines: usize) -> bool { true } -fn index_for_color(c: RgbColor) -> u8 { +fn index_for_color(c: Color) -> u8 { if c.is_named() || !(get_color_support().contains(ColorSupport::TERM_256COLOR)) { return c.to_name_index(); } @@ -421,10 +421,10 @@ pub struct Outputter { fd: RawFd, /// Foreground. - last_color: RgbColor, + last_color: Color, /// Background. - last_color2: RgbColor, + last_color2: Color, was_bold: bool, was_underline: bool, @@ -441,8 +441,8 @@ const fn new_from_fd(fd: RawFd) -> Self { contents: Vec::new(), buffer_count: 0, fd, - last_color: RgbColor::NORMAL, - last_color2: RgbColor::NORMAL, + last_color: Color::NORMAL, + last_color2: Color::NORMAL, was_bold: false, was_underline: false, was_italics: false, @@ -472,7 +472,7 @@ fn maybe_flush(&mut self) { /// Unconditionally write the color string to the output. /// Exported for builtin_set_color's usage only. - pub fn write_color(&mut self, color: RgbColor, is_fg: bool) -> bool { + pub fn write_color(&mut self, color: Color, is_fg: bool) -> bool { let supports_term24bit = get_color_support().contains(ColorSupport::TERM_24BIT); if !supports_term24bit || !color.is_rgb() { // Indexed or non-24 bit color. @@ -510,8 +510,8 @@ pub fn write_color(&mut self, color: RgbColor, is_fg: bool) -> bool { /// - 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 fn set_color(&mut self, mut fg: RgbColor, mut bg: RgbColor) { - const normal: RgbColor = RgbColor::NORMAL; + pub fn set_color(&mut self, mut fg: Color, mut bg: Color) { + const normal: Color = Color::NORMAL; let mut bg_set = false; let mut last_bg_set = false; let is_bold = fg.is_bold() || bg.is_bold(); @@ -557,10 +557,10 @@ pub fn set_color(&mut self, mut fg: RgbColor, mut bg: RgbColor) { // Background is set. bg_set = true; if fg == bg { - fg = if bg == RgbColor::WHITE { - RgbColor::BLACK + fg = if bg == Color::WHITE { + Color::BLACK } else { - RgbColor::WHITE + Color::WHITE }; } } @@ -576,7 +576,7 @@ pub fn set_color(&mut self, mut fg: RgbColor, mut bg: RgbColor) { self.reset_modes(); // We don't know if exit_attribute_mode resets colors, so we set it to something known. if write_foreground_color(self, 0) { - self.last_color = RgbColor::BLACK; + self.last_color = Color::BLACK; } } @@ -585,7 +585,7 @@ pub fn set_color(&mut self, mut fg: RgbColor, mut bg: RgbColor) { write_foreground_color(self, 0); self.write_command(ExitAttributeMode); - self.last_color2 = RgbColor::NORMAL; + self.last_color2 = Color::NORMAL; self.reset_modes(); } else if !fg.is_special() { self.write_color(fg, true /* foreground */); @@ -738,13 +738,13 @@ fn write_bytes(&mut self, buf: &[u8]) { /// Given a list of RgbColor, pick the "best" one, as determined by the color support. Returns /// RgbColor::NONE if empty. -pub fn best_color(candidates: &[RgbColor], support: ColorSupport) -> RgbColor { +pub fn best_color(candidates: &[Color], support: ColorSupport) -> Color { if candidates.is_empty() { - return RgbColor::NONE; + return Color::NONE; } - let mut first_rgb = RgbColor::NONE; - let mut first_named = RgbColor::NONE; + let mut first_rgb = Color::NONE; + let mut first_named = Color::NONE; for color in candidates { if first_rgb.is_none() && color.is_rgb() { first_rgb = *color; @@ -771,7 +771,7 @@ pub fn best_color(candidates: &[RgbColor], support: ColorSupport) -> RgbColor { /// Return the internal color code representing the specified color. /// TODO: This code should be refactored to enable sharing with builtin_set_color. /// In particular, the argument parsing still isn't fully capable. -pub fn parse_color(var: &EnvVar, is_background: bool) -> RgbColor { +pub fn parse_color(var: &EnvVar, is_background: bool) -> Color { let mut result = parse_color_maybe_none(var, is_background); if result.is_none() { result.typ = color::Type::Normal; @@ -779,14 +779,14 @@ pub fn parse_color(var: &EnvVar, is_background: bool) -> RgbColor { result } -pub fn parse_color_maybe_none(var: &EnvVar, is_background: bool) -> RgbColor { +pub fn parse_color_maybe_none(var: &EnvVar, is_background: bool) -> Color { let mut is_bold = false; let mut is_underline = false; let mut is_italics = false; let mut is_dim = false; let mut is_reverse = false; - let mut candidates: Vec = Vec::new(); + let mut candidates: Vec = Vec::new(); let prefix = L!("--background="); @@ -831,7 +831,7 @@ pub fn parse_color_maybe_none(var: &EnvVar, is_background: bool) -> RgbColor { } if !color_name.is_empty() { - let color: Option = RgbColor::from_wstr(&color_name); + let color: Option = Color::from_wstr(&color_name); if let Some(color) = color { candidates.push(color); }