Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Function to parse both hex and named colours from a string #306

Open
vi opened this issue Mar 1, 2023 · 8 comments
Open

Function to parse both hex and named colours from a string #306

vi opened this issue Mar 1, 2023 · 8 comments

Comments

@vi
Copy link

vi commented Mar 1, 2023

Description

There is named_from_str for looking up pre-defined colours by names. There is also a hard to find FromStr implementation for parsing hex colours.

I think there should be some easy function that does both - tries to decode a colour by name, then as hex colour and fails only if neither succeeds. This should allow for easier user-friendly colour specification in CLI or config files.

@Ogeon
Copy link
Owner

Ogeon commented Mar 1, 2023

Hi, I see your point and I definitely don't want it to be unnecessarily difficult. The problem is that there are so many ways of representing RGB as a string (CSS syntax, for example) so it's always going to be lacking something. I prefer to provide the parts separately and let the library user compose them as they see fit.

That said, do you think an explicit and more visible from_hex function could help instead?

@vi
Copy link
Author

vi commented Mar 1, 2023

Maybe. Also searching for "parse" on docs.rs should return something relevant. palette::rgb::FromHexError should also point to the place where such error may originate - this type is easier to find.

many ways of representing RGB as a string (CSS syntax, for example)

Maybe the docs can point and some external crates (compatible with palette) that does the parsing according to some standard.

@Ogeon
Copy link
Owner

Ogeon commented Mar 1, 2023

First, sorry for coming off as negative. It's good that you highlighted that it's inconvenient and hard to find everything that's available. Thank you!

The reason why I prefer to keep them separate is that I'm generally trying to move away from options that look like "one size fits all". They tend to end up becoming either lackluster or way too complex. Or both.

Also searching for "parse" on docs.rs should return something relevant. palette::rgb::FromHexError should also point to the place where such error may originate - this type is easier to find.

Definitely, I think having it as its own method will make cross referencing and attaching aliases more natural. And the named colors are really old code that hasn't yet been updated for better discoverability with newer rustdoc features. FromStr should really just be a shortcut to a default behavior.

Maybe the docs can point and some external crates (compatible with palette) that does the parsing according to some standard.

That could be nice, especially if there are some well established options. 🤔 I think people tend to make them quite generic so basically anything should work with some light glue code. Most crates I have seen can communicate in [T; 3] or (T, T, T).

@vi
Copy link
Author

vi commented Mar 1, 2023

patelle crate is in general rather generics-heavy, which makes it complicated, especially for newcomers to Rust.

Having more dedicated functions like to_hsv(), to_rgb(), to_rgba_u32() that does the same as already available generic functions, but with specific types hard coded can help with discoverability and ease of use in some situations. from_hex_str(&str) can fit this idea.

@Ogeon
Copy link
Owner

Ogeon commented Mar 2, 2023

That could also be helpful! The combinations are too many (potentially endless) to have exactly all of them but the most common cases can of course still be covered. 🙂 It can also be useful as a bit of a hint towards what's the more preferred or optimal option. For example, it's going to be possible to convert RGB straight from encoded u8 to linear f32 and it's faster than performing each step separately.

@Redhawk18
Copy link

That could also be helpful! The combinations are too many (potentially endless) to have exactly all of them but the most common cases can of course still be covered. 🙂 It can also be useful as a bit of a hint towards what's the more preferred or optimal option. For example, it's going to be possible to convert RGB straight from encoded u8 to linear f32 and it's faster than performing each step separately.

Hi, This my first time interacting with this crate's community and I'm working on a app with two frontends and I use palette for my colors. This is currently how I convert my hex colors into palette's RGBs.

fn hex_to_color(hex: &str) -> Option<Rgb> {
    if hex.len() == 7 {
        let hash = &hex[0..1];
        let red = u8::from_str_radix(&hex[1..3], 16);
        let green = u8::from_str_radix(&hex[3..5], 16);
        let blue = u8::from_str_radix(&hex[5..7], 16);

        return match (hash, red, green, blue) {
            ("#", Ok(red), Ok(green), Ok(blue)) => Some(Rgb::new(
                red as f32 / 255.0,
                green as f32 / 255.0,
                blue as f32 / 255.0,
            )),
            _ => None,
        };
    }

    None
}

At least hex colors should be natively parsed by the library. What's the state of this issue?

@Ogeon
Copy link
Owner

Ogeon commented Sep 23, 2023

Hi! You can already parse hex colors via the FromStr trait, so either of these should work:

let example1: Srgb<u8> = hex.parse().ok()?; // .ok() throws the error away and returns Option<Srgb<u8>>
let example2 = Srgb::<u8>::from_str(hex).ok()?;

Keep in mind that this doesn't require the # in the beginning, but still allows it. If you want to require it, you need to add that check.

I'm keeping this issue open as a reminder to consider adding an explicit from_hex method as well.

@Ogeon
Copy link
Owner

Ogeon commented Sep 23, 2023

As a first step toward making it more discoverable, I recently added more visible examples of parsing hex values in the documentation. It's not yet released, so here's a preview: https://ogeon.github.io/palette/palette/rgb/struct.Rgb.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants