sharkdp / pastel

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

Added traits: from and display to all color struct #133

Closed bresilla closed 3 years ago

bresilla commented 3 years ago

For each colorstruct (Color, Lab, RGB...) traits 'From' and 'Diplay' are coded. Those are very important when you want simply comvert from one color to another with standard Rust things: let new_color = Lab::from(&Color) or let new_color: Lab = somme_color.into()

In addidtion another delta_e was added: cir94, which is faster than ciede2000 bu more accurate than cie76.

Lastly, a function nearest was added that is 5 lines, but, with your premission in the future, i want to add functionality to extract colors fom image, and this function is needed. It simply finds the index and distance from nearest color from a group of colors

sharkdp commented 3 years ago

Thank you for your contribution!

All of these changes sound great, but I would prefer if we could split them into multiple PRs.

i want to add functionality to extract colors fom image, and this function is needed.

This sounds like a useful functionality, but I'm not sure it would really fit into this library. Wouldn't it require new dependencies?

bresilla commented 3 years ago

Yes sure. I think that delta_e changes can be ingnored for the moment. I can try later as anther PR for that (an loading colors from images - which requires only image lib).

The changes in lib file are that have the traits From and Display added. I don't know if you want this as two PR's (one for each)? Is that what you mean to split in multiple PR'S?

sharkdp commented 3 years ago

I don't know if you want this as two PR's (one for each)? Is that what you mean to split in multiple PR'S?

No. I Display and From is fine in one PR, but it would be great to split out the other things (nearest and new distance function)

bresilla commented 3 years ago

Sure thing. I'll do that.

PS: I am building a tool that uses extensively pastel, and along the way, if i see that some functions are better in pastel, I'll make a PR :)

sharkdp commented 3 years ago

PS: I am building a tool that uses extensively pastel, and along the way, if i see that some functions are better in pastel, I'll make a PR :)

Sounds great. Definitely interested to hear more :smile:

bresilla commented 3 years ago

You already have in the main function the print_XXX_string(), which in all cases is a formated version something like hsl({:.0},{space}{:.1}%,{space}{:.1}%). Do you think that the display funciton we should have more like hsl({}, {}, {}), so its more like raw numbers. SO when people want to use:

let new_color = Color::from(&RGBA{ r:100, g:100, b:100, alpha: 1.0} )

then this println!("{}", new_color.to_hsl_string()) would print better formmated version like:

hsl(0, 1.0, 0.5)

while this println!("{}", new_color) would print a rawer version:

hsl(0.0000, 1.0000, 0.5253)

So, if you use the function you build it, then you get a prettier format, but if you don't specify, the Display trait will print it a bit more accurate but less pretty.

sharkdp commented 3 years ago

So, if you use the function you build it, then you get a prettier format, but if you don't specify, the Display trait will print it a bit more accurate but less pretty.

Yes, I think that's a good idea. Just do the "bare minimum" work in the Display trait. It's meant for debugging, not for pretty-printing (to my knowledge).

sharkdp commented 3 years ago

Could you please also add an entry to the "unreleased" section in CHANGELOG.md? The format for entries is:

- Description what has been changed, see #123 (@user)

where #123 links to the bug ticket and/or the PR and user is your username.

bresilla commented 3 years ago

Could you please also add an entry to the "unreleased" section in CHANGELOG.md? The format for entries is:

Done. :)

So, if you use the function you build it, then you get a prettier format, but if you don't specify, the Display trait will print it a bit more accurate but less pretty.

Well, there is the Debug trait too but that is automatically implemented i believe, in the other hand the Display makes possible that any struct that implements can have to_string() which is very useful for others that want to use this library.

sharkdp commented 3 years ago

Thank you!