-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add :default to colorize and document ColorRGB, Color256 #6427
Conversation
Would be nice to document what actually is the default color. Or even remove it altogether since without context IMHO having default doesn't make much sense... |
With #6074 |
@Sija having puts "hello".colorize(some_bool ? :green : :default) so if some_bool
puts "hello".colorize(:green)
else
puts "hello"
end The @straight-shoota So you mean adding def colorize(fore : Colorize::ColorANSI)
Colorize::Object.new(self).fore(fore)
end would be better? |
Because of #6074, shouldn't we just remove all existing symbols in |
@Qwerp-Derp problem with #6074 is that it allows only passing symbol literals, it ain't work if you use variables for instance. |
@Sija Why would you ever use anything other than a symbol literal in this case? If you were to use a variable, why not just use the full name of the enum? a = Colorize::ColorANSI::Default
"foo".colorize(a) I agree something like Therefore, I still think we should get rid of all the symbols in COLORS, or at least change the |
Yeah, using enums now should be the way to go. |
I think Enums would be much better, because at the moment it's very easy to get runtime errors that should have been caught at compile-time:
|
I forgot to add This PR looks totally good and it should be merged ASAP. Using enum instead symbol is another and wider topic than this PR. Should talk in new issue. @r00ster91 Thank you! And sorry for missing this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @r00ster91 👍
I think it would be good when you can just write:
instead of: