sunjay / turtle

Create Animated Drawings in Rust
http://turtle.rs
Mozilla Public License 2.0
559 stars 54 forks source link

Move all color constants to a single module #187

Closed sunjay closed 3 years ago

sunjay commented 3 years ago

In #171, I suggested that we move the color constants from the color module into the Color struct itself. After merging #186, I spent some time thinking about it a bit more and wondered if we might want to take a different approach.

Here are some problems with how we currently define color constants and how we show them in the documentation:

To solve all of this, I am now suggesting that we move all of the color constants to a single color_consts module. This will allow us to make a single, complete list of all the supported color names and avoid filling the Color struct documentation with information that is rarely used.

To-do

Please don't hesitate to ask questions in the comments below or on Zulip!

cc @noeddl in case you're interested in doing a follow up from #186.

noeddl commented 3 years ago

@sunjay Yes, I would like to continue working on this.

noeddl commented 3 years ago

I have a question about this to-do:

change the macro that defines COLORS and COLOR_NAMES to define those constants within an impl Color block (this should create a single set of those constants that actually contains ALL the colors supported in this crate)

As we already noticed in the previous PR, defining these variables within an impl Color block won't work because static variables are not allowed as associated items in structs. So we should maybe continue using the functions that you came up with instead?

sunjay commented 3 years ago

As we already noticed in the previous PR, defining these variables within an impl Color block won't work because static variables are not allowed as associated items in structs. So we should maybe continue using the functions that you came up with instead?

Oops! You are totally right. Yes, please use the associated functions instead. Thanks!