Ogeon / palette

A Rust library for linear color calculations and conversion
Apache License 2.0
748 stars 60 forks source link

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

Open vi opened 1 year ago

vi commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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