sharkdp / pastel

A command-line tool to generate, analyze, convert and manipulate colors
Apache License 2.0
5.11k stars 104 forks source link

Support all CSS color formats #12

Closed sharkdp closed 4 years ago

sharkdp commented 5 years ago

See https://developer.mozilla.org/en-US/docs/Web/CSS/color_value for all details and test cases

optional:

saidsay-so commented 5 years ago

I implemented most of the functions here. Do I make one PR for all or a PR for each commit ?

sharkdp commented 5 years ago

One for all sounds great. Thank you very much.

saidsay-so commented 5 years ago

No problem!

sharkdp commented 5 years ago

After the updates by @MusiKid, we do now support almost every possible color format in the CSS spec.

While testing the examples given on the website mentioned above, I found two cases which were not supported yet:

Maybe we should try to solve these in a more general case (instead of adding even more regex patterns):

There were a few more cases which also did not work, but I don't think we have to necessarily support them (mostly involving alpha values).

halfbro commented 5 years ago

Hey, I'm interested in tackling the general parsing problem here. Do you think that using a parser combinator library (such as nom) would be a good solution here?

sharkdp commented 5 years ago

Hey, I'm interested in tackling the general parsing problem here

fantastic :+1:

Do you think that using a parser combinator library (such as nom) would be a good solution here?

That's what I was thinking about when writing "… instead of adding even more regex patterns" above :smile:.

I initially wondered if I should use a parser combinator library when writing the first few patterns, but I wasn't sure if it would definitely be the better solution. We certainly don't need all the power of a generic parser here. It's definitely manageable with a collection of regex patterns, but the code duplication isn't exactly great.

In short, I'd be in favor of a parser-combinator solution if:

halfbro commented 5 years ago

I started this last night, I got this far, using nom:

fn opt_hash_char(s: &str) -> IResult<&str, Option<char>> {
    opt(char('#'))(s)
}

fn parse_hex(input: &str) -> IResult<&str, Color> {
    let (input, _) = opt_hash_char(input)?;
    let (input, hex_chars) = hex_digit1(input)?;
    match hex_chars.len() {
        6 => {
            let r = hex_to_u8_unsafe(&hex_chars[0..2]);
            let g = hex_to_u8_unsafe(&hex_chars[2..4]);
            let b = hex_to_u8_unsafe(&hex_chars[4..6]);
            Ok((input, rgb(r, g, b)))
        }
        3 => {
            let r = hex_to_u8_unsafe(&hex_chars[0..1]);
            let g = hex_to_u8_unsafe(&hex_chars[1..2]);
            let b = hex_to_u8_unsafe(&hex_chars[2..3]);
            let r = r * 16 + r;
            let g = g * 16 + g;
            let b = b * 16 + b;
            Ok((input, rgb(r, g, b)))
        }
        _ => Err(Err::Error((
            "Expected hex string of 3 or 6 characters length",
            nom::error::ErrorKind::Many1,
        ))),
    }
}

Which handles all the hex rgb cases, AFAICT. A lot simpler than 2-3 regexes, imo. Using nom also allows us to get error messages about the input, if that seems useful.