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

less verbosity in ui::Color::Rgb{...} #41

Closed
adsick opened this issue Dec 11, 2021 · 4 comments
Closed

less verbosity in ui::Color::Rgb{...} #41

adsick opened this issue Dec 11, 2021 · 4 comments
Milestone

Comments

@adsick
Copy link

adsick commented Dec 11, 2021

I suggest replacing it with ui::Color::Rgb(u8, u8, u8)

@mikaelmello
Copy link
Owner

I agree it has its benefits. Using a struct provides better readability when reading the values (e.g. color.r instead of color.0), but I think only the underlying terminal library actually reads these values, while we create them. While your suggestion makes it much less verbose to create them :)

Would you be willing to make a PR?

@adsick
Copy link
Author

adsick commented Dec 14, 2021

Oh, I didn't think about color.r that's true. Idk, I'm kinda busy...

@tpoliaw
Copy link
Contributor

tpoliaw commented Jun 9, 2022

Would you ever be able to use color.r if the fields are only on one variant of the enum? To get the values out you'd need to do something like if let Color::Rgb{r, g, b} = color { ... } which could just as easily be if let Color::Rgb(r, g, b) = color { ... }.

That said, I quite like the explicitness of naming the fields here even when it might not be necessary. What about adding an rgb(u8, u8, u8) static method to Color that handles the verbosity when creating instances?

@mikaelmello
Copy link
Owner

Both your points are perfect to me. It can get cumbersome on pattern matching and the like but I quite like to be explicit as well. Let's go with the static method.

@mikaelmello mikaelmello modified the milestones: v1.0.0, v0.4.0 Aug 19, 2022
tpoliaw added a commit to tpoliaw/inquire that referenced this issue Aug 21, 2022
Allows an Rgb Color variant to be created as if it was a tuple variant
while maintaining the named fields of the struct type.

Fixes mikaelmello#41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

3 participants