From b00ee98fc4d910178ad89bd09fc6983f77cd7108 Mon Sep 17 00:00:00 2001 From: Tau Date: Sat, 16 Mar 2024 11:58:16 +0100 Subject: [PATCH 1/6] Move color parsing to separate module --- src/color.rs | 55 ------------------------------------------- src/lib.rs | 2 ++ src/xparsecolor.rs | 58 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 55 deletions(-) create mode 100644 src/xparsecolor.rs diff --git a/src/color.rs b/src/color.rs index 0f82ad7..ae7dc57 100644 --- a/src/color.rs +++ b/src/color.rs @@ -46,47 +46,6 @@ impl From for Color { } } -impl Color { - /// Parses an X11 color (see `man xparsecolor`). - #[cfg(unix)] - pub(crate) fn parse_x11(input: &str) -> Option { - let raw_parts = input.strip_prefix("rgb:")?; - let mut parts = raw_parts.split('/'); - let r = parse_channel(parts.next()?)?; - let g = parse_channel(parts.next()?)?; - let b = parse_channel(parts.next()?)?; - Some(Color { r, g, b }) - } - - // Some terminals (only Terminology found so far) respond with a - // CSS-like hex color code. - #[cfg(unix)] - pub(crate) fn parse_css_like(input: &str) -> Option { - let raw_parts = input.strip_prefix('#')?; - let len = raw_parts.len(); - if len == 6 { - let r = parse_channel(&raw_parts[..2])?; - let g = parse_channel(&raw_parts[2..4])?; - let b = parse_channel(&raw_parts[4..])?; - Some(Color { r, g, b }) - } else { - None - } - } -} - -#[cfg(unix)] -fn parse_channel(input: &str) -> Option { - let len = input.len(); - // From the xparsecolor man page: - // h indicates the value scaled in 4 bits, - // hh the value scaled in 8 bits, - // hhh the value scaled in 12 bits, and - // hhhh the value scaled in 16 bits, respectively. - let shift = (1..=4).contains(&len).then_some(16 - 4 * len as u16)?; - Some(u16::from_str_radix(input, 16).ok()? << shift) -} - // Implementation of determining the perceived lightness // follows this excellent answer: https://stackoverflow.com/a/56678483 @@ -116,7 +75,6 @@ fn luminance_to_perceived_lightness(luminance: f64) -> u8 { } #[cfg(test)] -#[allow(clippy::unwrap_used)] mod tests { use super::*; @@ -135,17 +93,4 @@ mod tests { }; assert_eq!(100, white.perceived_lightness()) } - - #[test] - #[cfg(unix)] - fn parses_css_like_color() { - assert_eq!( - Color { - r: 171 << 8, - g: 205 << 8, - b: 239 << 8 - }, - Color::parse_css_like("#ABCDEF").unwrap() - ) - } } diff --git a/src/lib.rs b/src/lib.rs index 5a5171c..7f60609 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -88,6 +88,8 @@ use thiserror::Error; mod color; mod os; +#[cfg(unix)] +mod xparsecolor; #[cfg(unix)] mod xterm; diff --git a/src/xparsecolor.rs b/src/xparsecolor.rs new file mode 100644 index 0000000..fe56945 --- /dev/null +++ b/src/xparsecolor.rs @@ -0,0 +1,58 @@ +use crate::Color; + +impl Color { + /// Parses an X11 color (see `man xparsecolor`). + pub(crate) fn parse_x11(input: &str) -> Option { + let raw_parts = input.strip_prefix("rgb:")?; + let mut parts = raw_parts.split('/'); + let r = parse_channel(parts.next()?)?; + let g = parse_channel(parts.next()?)?; + let b = parse_channel(parts.next()?)?; + Some(Color { r, g, b }) + } + + // Some terminals (only Terminology found so far) respond with a + // CSS-like hex color code. + pub(crate) fn parse_css_like(input: &str) -> Option { + let raw_parts = input.strip_prefix('#')?; + let len = raw_parts.len(); + if len == 6 { + let r = parse_channel(&raw_parts[..2])?; + let g = parse_channel(&raw_parts[2..4])?; + let b = parse_channel(&raw_parts[4..])?; + Some(Color { r, g, b }) + } else { + None + } + } +} + +#[cfg(unix)] +fn parse_channel(input: &str) -> Option { + let len = input.len(); + // From the xparsecolor man page: + // h indicates the value scaled in 4 bits, + // hh the value scaled in 8 bits, + // hhh the value scaled in 12 bits, and + // hhhh the value scaled in 16 bits, respectively. + let shift = (1..=4).contains(&len).then_some(16 - 4 * len as u16)?; + Some(u16::from_str_radix(input, 16).ok()? << shift) +} + +#[cfg(test)] +#[allow(clippy::unwrap_used)] +mod tests { + use super::*; + + #[test] + fn parses_css_like_color() { + assert_eq!( + Color { + r: 171 << 8, + g: 205 << 8, + b: 239 << 8 + }, + Color::parse_css_like("#ABCDEF").unwrap() + ) + } +} From cfc513d232335349f852911e148d946b0ff93a89 Mon Sep 17 00:00:00 2001 From: Tau Date: Sat, 16 Mar 2024 13:34:41 +0100 Subject: [PATCH 2/6] Rewrite parsing with correct scaling --- src/xparsecolor.rs | 222 +++++++++++++++++++++++++++++++++++++-------- src/xterm.rs | 3 +- 2 files changed, 185 insertions(+), 40 deletions(-) diff --git a/src/xparsecolor.rs b/src/xparsecolor.rs index fe56945..346c958 100644 --- a/src/xparsecolor.rs +++ b/src/xparsecolor.rs @@ -1,42 +1,90 @@ use crate::Color; -impl Color { - /// Parses an X11 color (see `man xparsecolor`). - pub(crate) fn parse_x11(input: &str) -> Option { - let raw_parts = input.strip_prefix("rgb:")?; - let mut parts = raw_parts.split('/'); - let r = parse_channel(parts.next()?)?; - let g = parse_channel(parts.next()?)?; - let b = parse_channel(parts.next()?)?; +/// Parses a color value that follows the `xparsecolor` format. +pub(crate) fn xparsecolor(input: &str) -> Option { + if input.starts_with('#') { + parse_sharp(&input[1..]) + } else if input.starts_with("rgb:") { + parse_rgb(&input[4..]) + } else if input.starts_with("rgbi:") { + parse_rgbi(&input[5..]) + } else { + None + } +} + +/// From the `xparsecolor` man page: +/// > For backward compatibility, an older syntax for RGB Device is supported, +/// > but its continued use is not encouraged. The syntax is an initial sharp sign character +/// > followed by a numeric specification, in one of the following formats: +/// > +/// > The R, G, and B represent single hexadecimal digits. +/// > When fewer than 16 bits each are specified, they represent the most significant bits of the value +/// > (unlike the `rgb:` syntax, in which values are scaled). +/// > For example, the string `#3a7` is the same as `#3000a0007000`. +fn parse_sharp(input: &str) -> Option { + const NUM_COMPONENTS: usize = 3; + let len = input.len(); + if len % NUM_COMPONENTS == 0 && len <= NUM_COMPONENTS * 4 { + let chunk_size = input.len() / NUM_COMPONENTS; + let r = parse_channel_shifted(&input[0..chunk_size])?; + let g = parse_channel_shifted(&input[chunk_size..chunk_size * 2])?; + let b = parse_channel_shifted(&input[chunk_size * 2..])?; Some(Color { r, g, b }) + } else { + None } +} + +fn parse_channel_shifted(input: &str) -> Option { + let value = u16::from_str_radix(input, 16).ok()?; + Some(value << (4 - input.len()) * 4) +} - // Some terminals (only Terminology found so far) respond with a - // CSS-like hex color code. - pub(crate) fn parse_css_like(input: &str) -> Option { - let raw_parts = input.strip_prefix('#')?; - let len = raw_parts.len(); - if len == 6 { - let r = parse_channel(&raw_parts[..2])?; - let g = parse_channel(&raw_parts[2..4])?; - let b = parse_channel(&raw_parts[4..])?; - Some(Color { r, g, b }) - } else { - None - } +/// From the `xparsecolor` man page: +/// > An RGB Device specification is identified by the prefix `rgb:` and conforms to the following syntax: +/// > ```text +/// > rgb:// +/// > +/// > , , := h | hh | hhh | hhhh +/// > h := single hexadecimal digits (case insignificant) +/// > ``` +/// > Note that *h* indicates the value scaled in 4 bits, +/// > *hh* the value scaled in 8 bits, *hhh* the value scaled in 12 bits, +/// > and *hhhh* the value scaled in 16 bits, respectively. +fn parse_rgb(input: &str) -> Option { + let mut parts = input.split('/'); + let r = parse_channel_scaled(parts.next()?)?; + let g = parse_channel_scaled(parts.next()?)?; + let b = parse_channel_scaled(parts.next()?)?; + if parts.next().is_none() { + Some(Color { r, g, b }) + } else { + None } } -#[cfg(unix)] -fn parse_channel(input: &str) -> Option { +fn parse_channel_scaled(input: &str) -> Option { let len = input.len(); - // From the xparsecolor man page: - // h indicates the value scaled in 4 bits, - // hh the value scaled in 8 bits, - // hhh the value scaled in 12 bits, and - // hhhh the value scaled in 16 bits, respectively. - let shift = (1..=4).contains(&len).then_some(16 - 4 * len as u16)?; - Some(u16::from_str_radix(input, 16).ok()? << shift) + if (1..=4).contains(&len) { + let max = u32::pow(16, len as u32) - 1; + let value = u32::from_str_radix(input, 16).ok()?; + Some((u16::MAX as u32 * value / max) as u16) + } else { + None + } +} + +/// From the `xparsecolor` man page: +/// > An RGB intensity specification is identified by the prefix `rgbi:` and conforms to the following syntax: +/// > ```text +/// > rgbi:// +/// > ``` +/// Note that red, green, and blue are floating-point values between 0.0 and 1.0, inclusive. +/// The input format for these values is an optional sign, a string of numbers possibly containing a decimal point, +/// and an optional exponent field containing an E or e followed by a possibly signed integer string. +fn parse_rgbi(_input: &str) -> Option { + todo!() } #[cfg(test)] @@ -44,15 +92,111 @@ fn parse_channel(input: &str) -> Option { mod tests { use super::*; + // Tests adapted from alacritty/vte: + // https://github.com/alacritty/vte/blob/ed51aa19b7ad060f62a75ec55ebb802ced850b1a/src/ansi.rs#L2134 + #[test] + fn parses_valid_rgb_color() { + assert_eq!( + xparsecolor("rgb:f/e/d"), + Some(Color { + r: 0xffff, + g: 0xeeee, + b: 0xdddd, + }) + ); + assert_eq!( + xparsecolor("rgb:11/aa/ff"), + Some(Color { + r: 0x1111, + g: 0xaaaa, + b: 0xffff + }) + ); + assert_eq!( + xparsecolor("rgb:f/ed1/cb23"), + Some(Color { + r: 0xffff, + g: 0xed1d, + b: 0xcb23, + }) + ); + assert_eq!( + xparsecolor("rgb:ffff/0/0"), + Some(Color { + r: 0xffff, + g: 0x0, + b: 0x0 + }) + ); + } + + #[test] + fn fails_for_invalid_rgb_color() { + assert!(xparsecolor("rgb:").is_none()); // Empty + assert!(xparsecolor("rgb:f/f").is_none()); // Not enough channels + assert!(xparsecolor("rgb:f/f/f/f").is_none()); // Too many channels + assert!(xparsecolor("rgb:f//f").is_none()); // Empty channel + assert!(xparsecolor("rgb:ffff/ffff/fffff").is_none()); // Too many digits for one channel + } + + // Tests adapted from alacritty/vte: + // https://github.com/alacritty/vte/blob/ed51aa19b7ad060f62a75ec55ebb802ced850b1a/src/ansi.rs#L2142 + #[test] + fn parses_valid_sharp_color() { + assert_eq!( + xparsecolor("#1af"), + Some(Color { + r: 0x1000, + g: 0xa000, + b: 0xf000, + }) + ); + assert_eq!( + xparsecolor("#1AF"), + Some(Color { + r: 0x1000, + g: 0xa000, + b: 0xf000, + }) + ); + assert_eq!( + xparsecolor("#11aaff"), + Some(Color { + r: 0x1100, + g: 0xaa00, + b: 0xff00 + }) + ); + assert_eq!( + xparsecolor("#110aa0ff0"), + Some(Color { + r: 0x1100, + g: 0xaa00, + b: 0xff00 + }) + ); + assert_eq!( + xparsecolor("#1100aa00ff00"), + Some(Color { + r: 0x1100, + g: 0xaa00, + b: 0xff00 + }) + ); + assert_eq!( + xparsecolor("#123456789ABC"), + Some(Color { + r: 0x1234, + g: 0x5678, + b: 0x9ABC + }) + ); + } + #[test] - fn parses_css_like_color() { - assert_eq!( - Color { - r: 171 << 8, - g: 205 << 8, - b: 239 << 8 - }, - Color::parse_css_like("#ABCDEF").unwrap() - ) + fn fails_for_invalid_sharp_color() { + assert!(xparsecolor("#").is_none()); // Empty + assert!(xparsecolor("#1234").is_none()); // Not divisible by three + assert!(xparsecolor("#123456789ABCDEF").is_none()); // Too many components } } diff --git a/src/xterm.rs b/src/xterm.rs index e23f33c..c89eb51 100644 --- a/src/xterm.rs +++ b/src/xterm.rs @@ -1,4 +1,5 @@ use self::io_utils::{read_until2, TermReader}; +use crate::xparsecolor::xparsecolor; use crate::{Color, ColorScheme, Error, QueryOptions, Result}; use std::env; use std::io::{self, BufRead, BufReader, Write as _}; @@ -74,7 +75,7 @@ fn parse_response(response: String, prefix: &str) -> Result { .strip_suffix('\x07') .or(response.strip_suffix("\x1b\\")) }) - .and_then(|c| Color::parse_x11(c).or_else(|| Color::parse_css_like(c))) + .and_then(xparsecolor) .ok_or_else(|| Error::Parse(response)) } From 08cc2177829d1976b26f7767e2da14f051861915 Mon Sep 17 00:00:00 2001 From: Tau Date: Sat, 16 Mar 2024 13:56:55 +0100 Subject: [PATCH 3/6] Improve docs --- doc/terminal-survey.md | 8 ++++++-- src/xparsecolor.rs | 21 ++++++--------------- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/doc/terminal-survey.md b/doc/terminal-survey.md index 9898bd0..b67035c 100644 --- a/doc/terminal-survey.md +++ b/doc/terminal-survey.md @@ -37,11 +37,15 @@ A list of terminals that were tested for support of DA1 (`CSI c`) and `OSC 10` /
-**ℹ️ Note:** +**ℹ️ Note 1:** Some Linux terminals are omitted since they all use the `vte` library behind the scenes. \ Here's a non-exhaustive list: GNOME Terminal, (GNOME) Console, MATE Terminal, XFCE Terminal, (GNOME) Builder, (elementary) Terminal, LXTerminal, Guake. -[^1]: The response does not use the `XParseColor` format but rather a CSS-like hex code (e.g. `#AAAAAA`). +**ℹ️ Note 2:** +If not otherwise noted, the terminals respond using the `rgb:r(rrr)/g(ggg)/b(bbbb)` format. +See [Color Strings](https://www.x.org/releases/current/doc/libX11/libX11/libX11.html#Color_Strings) for details on what is theoretically possible. + +[^1]: Responds using the `#r(rrr)g(ggg)b(bbb)` format. [Contour]: https://contour-terminal.org/ [QTerminal]: https://github.com/lxqt/qterminal diff --git a/src/xparsecolor.rs b/src/xparsecolor.rs index 346c958..575af47 100644 --- a/src/xparsecolor.rs +++ b/src/xparsecolor.rs @@ -1,13 +1,16 @@ use crate::Color; -/// Parses a color value that follows the `xparsecolor` format. +/// Parses a color value that follows the `XParseColor` format. +/// See https://www.x.org/releases/current/doc/libX11/libX11/libX11.html#Color_Strings +/// for a reference of what `XParseColor` supports. +/// +/// Not all formats are supported, just the ones that are returned +/// by the tested terminals. pub(crate) fn xparsecolor(input: &str) -> Option { if input.starts_with('#') { parse_sharp(&input[1..]) } else if input.starts_with("rgb:") { parse_rgb(&input[4..]) - } else if input.starts_with("rgbi:") { - parse_rgbi(&input[5..]) } else { None } @@ -75,18 +78,6 @@ fn parse_channel_scaled(input: &str) -> Option { } } -/// From the `xparsecolor` man page: -/// > An RGB intensity specification is identified by the prefix `rgbi:` and conforms to the following syntax: -/// > ```text -/// > rgbi:// -/// > ``` -/// Note that red, green, and blue are floating-point values between 0.0 and 1.0, inclusive. -/// The input format for these values is an optional sign, a string of numbers possibly containing a decimal point, -/// and an optional exponent field containing an E or e followed by a possibly signed integer string. -fn parse_rgbi(_input: &str) -> Option { - todo!() -} - #[cfg(test)] #[allow(clippy::unwrap_used)] mod tests { From 6d68b100409232bcfbd11a01ba7f2394958eb8e5 Mon Sep 17 00:00:00 2001 From: Tau Date: Sat, 16 Mar 2024 14:11:26 +0100 Subject: [PATCH 4/6] Encourage contributions --- src/xparsecolor.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/xparsecolor.rs b/src/xparsecolor.rs index 575af47..eec244a 100644 --- a/src/xparsecolor.rs +++ b/src/xparsecolor.rs @@ -5,7 +5,8 @@ use crate::Color; /// for a reference of what `XParseColor` supports. /// /// Not all formats are supported, just the ones that are returned -/// by the tested terminals. +/// by the tested terminals. Feel free to open a PR if you encounter +/// a terminal that returns a different format. pub(crate) fn xparsecolor(input: &str) -> Option { if input.starts_with('#') { parse_sharp(&input[1..]) From db30acfffe444425be4dacaf882b45bc901da6a4 Mon Sep 17 00:00:00 2001 From: Tau Date: Sat, 16 Mar 2024 14:14:32 +0100 Subject: [PATCH 5/6] Remove warning suppression --- src/xparsecolor.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/xparsecolor.rs b/src/xparsecolor.rs index eec244a..ee9130a 100644 --- a/src/xparsecolor.rs +++ b/src/xparsecolor.rs @@ -80,7 +80,6 @@ fn parse_channel_scaled(input: &str) -> Option { } #[cfg(test)] -#[allow(clippy::unwrap_used)] mod tests { use super::*; From d0ab91c55420049d76128b7e9be6581509aaaa2a Mon Sep 17 00:00:00 2001 From: Tau Date: Sat, 16 Mar 2024 14:15:31 +0100 Subject: [PATCH 6/6] Make clippy happy --- src/xparsecolor.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/xparsecolor.rs b/src/xparsecolor.rs index ee9130a..7f8613f 100644 --- a/src/xparsecolor.rs +++ b/src/xparsecolor.rs @@ -8,10 +8,10 @@ use crate::Color; /// by the tested terminals. Feel free to open a PR if you encounter /// a terminal that returns a different format. pub(crate) fn xparsecolor(input: &str) -> Option { - if input.starts_with('#') { - parse_sharp(&input[1..]) - } else if input.starts_with("rgb:") { - parse_rgb(&input[4..]) + if let Some(stripped) = input.strip_prefix('#') { + parse_sharp(stripped) + } else if let Some(stripped) = input.strip_prefix("rgb:") { + parse_rgb(stripped) } else { None } @@ -42,7 +42,7 @@ fn parse_sharp(input: &str) -> Option { fn parse_channel_shifted(input: &str) -> Option { let value = u16::from_str_radix(input, 16).ok()?; - Some(value << (4 - input.len()) * 4) + Some(value << ((4 - input.len()) * 4)) } /// From the `xparsecolor` man page: