Support hyphenated colour names in Color::from_str

[orf]

Problem

The Color::from_str implementation does not support hyphenated colour names like light-red.

We would love to use this method in gping (ref orf/gping#331 and orf/gping#332), but we have historically used hyphenated names.

I'd have assumed light-red is more common than lightred, but perhaps I'm wrong?

Solution

It would be great to also support hyphenated colour names here, so that light-red, lightred and light red all parse to the same Colour value.

I'm happy to make a PR (or maybe @TieWay59 might be interested) if this is accepted?

[TieWay59]

I did a little search, the naming convention can vary a lot:

Python click https://github.com/pallets/click/blob/d9af5cfa009c927a96d10ed38a3e37979876a12e/src/click/termui.py#L488-L506

    Supported color names:

    * ``black`` (might be a gray)
    * ``red``
    * ``green``
    * ``yellow`` (might be an orange)
    * ``blue``
    * ``magenta``
    * ``cyan``
    * ``white`` (might be light gray)
    * ``bright_black``
    * ``bright_red``
    * ``bright_green``
    * ``bright_yellow``
    * ``bright_blue``
    * ``bright_magenta``
    * ``bright_cyan``
    * ``bright_white``
    * ``reset`` (reset the color code only)

Rust crossterm https://github.com/crossterm-rs/crossterm/blob/1efdce7ef63cba6992729db5f22262a60936fa8b/src/style/types/color.rs#L381C13-L408C11

        assert_eq!(
            serde_json::from_str::<Color>("\"red\"").unwrap(),
            Color::Red
        );
        assert_eq!(
            serde_json::from_str::<Color>("\"dark_red\"").unwrap(),
            Color::DarkRed
        );
        assert_eq!(
            serde_json::from_str::<Color>("\"grey\"").unwrap(),
            Color::Grey
        );
        assert_eq!(
            serde_json::from_str::<Color>("\"dark_grey\"").unwrap(),
            Color::DarkGrey
        );
        assert_eq!(
            serde_json::from_str::<Color>("\"green\"").unwrap(),
            Color::Green
        );
        assert_eq!(
            serde_json::from_str::<Color>("\"dark_green\"").unwrap(),
            Color::DarkGreen
        );
        assert_eq!(
            serde_json::from_str::<Color>("\"yellow\"").unwrap(),
            Color::Yellow
        );

rust termion https://github.com/redox-os/termion/blob/f2b8517c3185d8a6384109c7309589aa9ad48b49/src/color.rs#L62-L77

derive_color!("Black.", Black, "0");
derive_color!("Red.", Red, "1");
derive_color!("Green.", Green, "2");
derive_color!("Yellow.", Yellow, "3");
derive_color!("Blue.", Blue, "4");
derive_color!("Magenta.", Magenta, "5");
derive_color!("Cyan.", Cyan, "6");
derive_color!("White.", White, "7");
derive_color!("High-intensity light black.", LightBlack, "8");
derive_color!("High-intensity light red.", LightRed, "9");
derive_color!("High-intensity light green.", LightGreen, "10");
derive_color!("High-intensity light yellow.", LightYellow, "11");
derive_color!("High-intensity light blue.", LightBlue, "12");
derive_color!("High-intensity light magenta.", LightMagenta, "13");
derive_color!("High-intensity light cyan.", LightCyan, "14");
derive_color!("High-intensity light white.", LightWhite, "15");

[joshka]

Seems reasonable - I don't think there is a consensus on which naming to use for these colors, which is a shame.

In various libs and docs I've seen light / bright / dark.
There's no clear consensus on what to name Gray / Silver / White, White / LightGray / Light White, DarkGray / LightBlack, on whether to use light vs bright, and how to spell grey / gray.

It's probably worth adding a small table to the docs to show how the color names that are parsed map to the ratatui colors and the various underlying libraries (I think there might be a bug or two in there for some backends, but it's hard to tell).

For the implementation, perhaps just drop any _, -, , convert bright -> light and parse from there?

[TieWay59]

Seems reasonable - I don't think there is a consensus on which naming to use for these colors, which is a shame.

In various libs and docs I've seen light / bright / dark. There's no clear consensus on what to name Gray / Silver / White, White / LightGray / Light White, DarkGray / LightBlack, on whether to use light vs bright, and how to spell grey / gray.

It's probably worth adding a small table to the docs to show how the color names that are parsed map to the ratatui colors and the various underlying libraries (I think there might be a bug or two in there for some backends, but it's hard to tell).

For the implementation, perhaps just drop any _, -, , convert bright -> light and parse from there?

Since this case can be touched by very few people, I agree with your plan which may have covered 99% of situations a user might type.

If everything is fine, how about labeling this issue as one good first issue?

[joshka]

If everything is fine, how about labeling this issue as one good first issue?

Apologies, I saw this after I was done. That would have been a good idea though :)

[TieWay59]

If everything is fine, how about labeling this issue as one good first issue?

Apologies, I saw this after I was done. That would have been a good idea though :)

Never mind, I wish we didn't mean to push it too hard, I thought this is a trivial issue ~

[joshka]

No problem - it was a definitely good idea to make some issues available for new contributors