From 417279e01b17c4da4d432f4ccd09870cf033bd0b Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Thu, 9 Nov 2023 17:45:05 -0500 Subject: [PATCH 1/3] Add support for parsing 3-value and 4-value hex codes --- crates/gpui2/src/color.rs | 103 +++++++++++++++++++++++++++++--------- 1 file changed, 80 insertions(+), 23 deletions(-) diff --git a/crates/gpui2/src/color.rs b/crates/gpui2/src/color.rs index 3ae247962b13071c0e7e711778f6d1c9b60dc9df..a49b372e869a2c76fb0cbcf84334b818b5ddef88 100644 --- a/crates/gpui2/src/color.rs +++ b/crates/gpui2/src/color.rs @@ -19,7 +19,7 @@ pub fn rgba(hex: u32) -> Rgba { Rgba { r, g, b, a } } -#[derive(Clone, Copy, Default)] +#[derive(PartialEq, Clone, Copy, Default)] pub struct Rgba { pub r: f32, pub g: f32, @@ -70,20 +70,11 @@ impl<'de> Visitor<'de> for RgbaVisitor { } fn visit_str(self, value: &str) -> Result { - if value.len() == 7 || value.len() == 9 { - let r = u8::from_str_radix(&value[1..3], 16).unwrap() as f32 / 255.0; - let g = u8::from_str_radix(&value[3..5], 16).unwrap() as f32 / 255.0; - let b = u8::from_str_radix(&value[5..7], 16).unwrap() as f32 / 255.0; - let a = if value.len() == 9 { - u8::from_str_radix(&value[7..9], 16).unwrap() as f32 / 255.0 - } else { - 1.0 - }; - Ok(Rgba { r, g, b, a }) - } else { - Err(E::custom( - "Bad format for RGBA. Expected #rrggbb or #rrggbbaa.", - )) + match value.len() { + 4 | 5 | 7 | 9 => Rgba::try_from(value).map_err(E::custom), + _ => Err(E::custom( + "Bad format for RGBA. Expected #rgb, #rgba, #rrggbb, or #rrggbbaa.", + )), } } } @@ -128,16 +119,47 @@ impl TryFrom<&'_ str> for Rgba { type Error = ParseIntError; fn try_from(value: &'_ str) -> Result { - let r = u8::from_str_radix(&value[1..3], 16)? as f32 / 255.0; - let g = u8::from_str_radix(&value[3..5], 16)? as f32 / 255.0; - let b = u8::from_str_radix(&value[5..7], 16)? as f32 / 255.0; - let a = if value.len() > 7 { - u8::from_str_radix(&value[7..9], 16)? as f32 / 255.0 - } else { - 1.0 + const RGB: usize = "rgb".len(); + const RGBA: usize = "rgba".len(); + const RRGGBB: usize = "rrggbb".len(); + const RRGGBBAA: usize = "rrggbbaa".len(); + + let Some(("", hex)) = value.split_once('#') else { + panic!("invalid color: {value}"); + }; + + let (r, g, b, a) = match hex.len() { + RGB | RGBA => { + let r = u8::from_str_radix(&hex[0..1].repeat(2), 16)?; + let g = u8::from_str_radix(&hex[1..2].repeat(2), 16)?; + let b = u8::from_str_radix(&hex[2..3].repeat(2), 16)?; + let a = if hex.len() == RGBA { + u8::from_str_radix(&hex[3..4].repeat(2), 16)? + } else { + 0xff + }; + (r, g, b, a) + } + RRGGBB | RRGGBBAA => { + let r = u8::from_str_radix(&hex[0..2], 16)?; + let g = u8::from_str_radix(&hex[2..4], 16)?; + let b = u8::from_str_radix(&hex[4..6], 16)?; + let a = if hex.len() == RRGGBBAA { + u8::from_str_radix(&hex[6..8], 16)? + } else { + 0xff + }; + (r, g, b, a) + } + _ => panic!("invalid color: {value}"), }; - Ok(Rgba { r, g, b, a }) + Ok(Rgba { + r: r as f32 / 255., + g: g as f32 / 255., + b: b as f32 / 255., + a: a as f32 / 255., + }) } } @@ -311,3 +333,38 @@ impl<'de> Deserialize<'de> for Hsla { Ok(Hsla::from(rgba)) } } + +#[cfg(test)] +mod tests { + use serde_json::json; + + use super::*; + + #[test] + fn test_deserialize_three_value_hex_to_rgba() { + let actual: Rgba = serde_json::from_value(json!("#f09")).unwrap(); + + assert_eq!(actual, rgba(0xff0099ff)) + } + + #[test] + fn test_deserialize_four_value_hex_to_rgba() { + let actual: Rgba = serde_json::from_value(json!("#f09f")).unwrap(); + + assert_eq!(actual, rgba(0xff0099ff)) + } + + #[test] + fn test_deserialize_six_value_hex_to_rgba() { + let actual: Rgba = serde_json::from_value(json!("#ff0099")).unwrap(); + + assert_eq!(actual, rgba(0xff0099ff)) + } + + #[test] + fn test_deserialize_eight_value_hex_to_rgba() { + let actual: Rgba = serde_json::from_value(json!("#ff0099ff")).unwrap(); + + assert_eq!(actual, rgba(0xff0099ff)) + } +} From 8f5adeb9c382eddd8c7c658383d74950db46e409 Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Thu, 9 Nov 2023 18:00:17 -0500 Subject: [PATCH 2/3] Improve error conditions when parsing hex colors --- crates/gpui2/src/color.rs | 37 ++++++++++++++++++++--------- crates/theme2/src/default_colors.rs | 6 ++--- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/crates/gpui2/src/color.rs b/crates/gpui2/src/color.rs index a49b372e869a2c76fb0cbcf84334b818b5ddef88..781551223f9daf4f35a9125e1474f2d0a63dbbd5 100644 --- a/crates/gpui2/src/color.rs +++ b/crates/gpui2/src/color.rs @@ -1,8 +1,8 @@ #![allow(dead_code)] +use anyhow::bail; use serde::de::{self, Deserialize, Deserializer, Visitor}; use std::fmt; -use std::num::ParseIntError; pub fn rgb>(hex: u32) -> C { let r = ((hex >> 16) & 0xFF) as f32 / 255.0; @@ -70,12 +70,7 @@ impl<'de> Visitor<'de> for RgbaVisitor { } fn visit_str(self, value: &str) -> Result { - match value.len() { - 4 | 5 | 7 | 9 => Rgba::try_from(value).map_err(E::custom), - _ => Err(E::custom( - "Bad format for RGBA. Expected #rgb, #rgba, #rrggbb, or #rrggbbaa.", - )), - } + Rgba::try_from(value).map_err(E::custom) } } @@ -116,7 +111,7 @@ impl From for Rgba { } impl TryFrom<&'_ str> for Rgba { - type Error = ParseIntError; + type Error = anyhow::Error; fn try_from(value: &'_ str) -> Result { const RGB: usize = "rgb".len(); @@ -124,12 +119,18 @@ impl TryFrom<&'_ str> for Rgba { const RRGGBB: usize = "rrggbb".len(); const RRGGBBAA: usize = "rrggbbaa".len(); - let Some(("", hex)) = value.split_once('#') else { - panic!("invalid color: {value}"); + const EXPECTED_FORMATS: &'static str = "Expected #rgb, #rgba, #rrggbb, or #rrggbbaa"; + + let Some(("", hex)) = value.trim().split_once('#') else { + bail!("invalid RGBA hex color: '{value}'. {EXPECTED_FORMATS}"); }; let (r, g, b, a) = match hex.len() { RGB | RGBA => { + // There's likely a better way to handle 3 and 4-value hex codes without + // needing to use `.repeat` and incur additional allocations. + // What we probably want to do is parse the single hex digit and then perform + // bit-shifting to "duplicate" the digit. let r = u8::from_str_radix(&hex[0..1].repeat(2), 16)?; let g = u8::from_str_radix(&hex[1..2].repeat(2), 16)?; let b = u8::from_str_radix(&hex[2..3].repeat(2), 16)?; @@ -151,7 +152,7 @@ impl TryFrom<&'_ str> for Rgba { }; (r, g, b, a) } - _ => panic!("invalid color: {value}"), + _ => bail!("invalid RGBA hex color: '{value}'. {EXPECTED_FORMATS}"), }; Ok(Rgba { @@ -367,4 +368,18 @@ mod tests { assert_eq!(actual, rgba(0xff0099ff)) } + + #[test] + fn test_deserialize_eight_value_hex_with_padding_to_rgba() { + let actual: Rgba = serde_json::from_value(json!(" #f5f5f5ff ")).unwrap(); + + assert_eq!(actual, rgba(0xf5f5f5ff)) + } + + #[test] + fn test_deserialize_eight_value_hex_with_mixed_case_to_rgba() { + let actual: Rgba = serde_json::from_value(json!("#DeAdbEeF")).unwrap(); + + assert_eq!(actual, rgba(0xdeadbeef)) + } } diff --git a/crates/theme2/src/default_colors.rs b/crates/theme2/src/default_colors.rs index 92e38ed39e6cc938b02b888e12f5fc5bfbf6daa1..6cfda37a2a632a2d7228152409c1deb1c00c2aa5 100644 --- a/crates/theme2/src/default_colors.rs +++ b/crates/theme2/src/default_colors.rs @@ -1,5 +1,3 @@ -use std::num::ParseIntError; - use gpui::{hsla, Hsla, Rgba}; use crate::colors::{StatusColors, SystemColors, ThemeColors}; @@ -413,10 +411,10 @@ struct StaticColorScaleSet { } impl TryFrom for ColorScaleSet { - type Error = ParseIntError; + type Error = anyhow::Error; fn try_from(value: StaticColorScaleSet) -> Result { - fn to_color_scale(scale: StaticColorScale) -> Result { + fn to_color_scale(scale: StaticColorScale) -> Result { scale .into_iter() .map(|color| Rgba::try_from(color).map(Hsla::from)) From 82861e3123c4b83ff4fb861d938679384e90282e Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Thu, 9 Nov 2023 18:19:58 -0500 Subject: [PATCH 3/3] Improve digit duplication --- crates/gpui2/src/color.rs | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/crates/gpui2/src/color.rs b/crates/gpui2/src/color.rs index 781551223f9daf4f35a9125e1474f2d0a63dbbd5..6fcb12e178991d2afb36b3f5d7e342abdd85550a 100644 --- a/crates/gpui2/src/color.rs +++ b/crates/gpui2/src/color.rs @@ -127,19 +127,22 @@ impl TryFrom<&'_ str> for Rgba { let (r, g, b, a) = match hex.len() { RGB | RGBA => { - // There's likely a better way to handle 3 and 4-value hex codes without - // needing to use `.repeat` and incur additional allocations. - // What we probably want to do is parse the single hex digit and then perform - // bit-shifting to "duplicate" the digit. - let r = u8::from_str_radix(&hex[0..1].repeat(2), 16)?; - let g = u8::from_str_radix(&hex[1..2].repeat(2), 16)?; - let b = u8::from_str_radix(&hex[2..3].repeat(2), 16)?; + let r = u8::from_str_radix(&hex[0..1], 16)?; + let g = u8::from_str_radix(&hex[1..2], 16)?; + let b = u8::from_str_radix(&hex[2..3], 16)?; let a = if hex.len() == RGBA { - u8::from_str_radix(&hex[3..4].repeat(2), 16)? + u8::from_str_radix(&hex[3..4], 16)? } else { - 0xff + 0xf }; - (r, g, b, a) + + /// Duplicates a given hex digit. + /// E.g., `0xf` -> `0xff`. + const fn duplicate(value: u8) -> u8 { + value << 4 | value + } + + (duplicate(r), duplicate(g), duplicate(b), duplicate(a)) } RRGGBB | RRGGBBAA => { let r = u8::from_str_radix(&hex[0..2], 16)?;